Add stats/roivolumes module and GM volumes support in atlas_roimetrics - #375
Add stats/roivolumes module and GM volumes support in atlas_roimetrics#375baptisp wants to merge 24 commits into
Conversation
…metrics - New module stats/roivolumes: computes voxel count and volume (mm³) per WM bundle mask or per GM label region using nibabel/numpy (no external tools). Supports both binary-mask mode (WM) and label-atlas mode (GM). - atlas_roimetrics subworkflow: adds optional GM Desikan parcellation ROI metrics (STATS_GM_ROIMETRICS) and optional volume computation for both WM bundles (STATS_WM_VOLUMES) and GM regions (STATS_GM_VOLUMES), gated by options.run_gm_roimetrics and options.run_roi_volumes respectively. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace non-existent IFGWM_uni.nii.gz with IFGWM_labels_map.nii.gz - Use checkIfExists:false for stub test (params.modules_testdata_base_path not set in nf-neuro repo) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…traction When run_roi_metrics = false, STATS_METRICSINROI and STATS_GM_ROIMETRICS are skipped. Atlas registration and volume computation (STATS_WM_VOLUMES, STATS_GM_VOLUMES) still run as normal. This allows volumes-only runs without waiting for diffusion metrics extraction. Default is true for backward compatibility. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…i_metrics to meta.yml Document the three options added with GM parcellation and volume features so UTILS_OPTIONS recognises them and does not emit unknown-option warnings. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… tab bug <<-END_VERSIONS strips leading tabs, not spaces. This module uses space indentation so the END_VERSIONS terminator was leaked into versions.yml, causing a YAML parse error at pipeline teardown. Write versions.yml inside the Python block instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lumns Previously, meta.id and meta.session were concatenated via ext.prefix, and the BIDS session prefix from warped mask filenames bled into the bundle/region name. Changes: - WM output: subject_id, session, bundle, volume_voxels, volume_mm3 (bundle name stripped of session prefix from ANTSAPPLYTRANSFORMS output) - GM output: subject_id, session, region, volume_voxels, volume_mm3 (subject_id and session split from the fused prefix string) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sion The 'sample' column contained the fused ext.prefix (e.g. sub-XXX_ses-YYYYMMDD). Replace it with two standard BIDS columns: subject_id (meta.id) | session (meta.session) | roi | metrics... Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add meta.run as a dedicated 'run' column in both metricsinroi and roivolumes outputs. Column is empty when no run is present in meta, which is the common case. This disambiguates datasets where multiple T1/DWI runs exist within the same session. Column order: subject_id | session | run | roi/bundle/region | values Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ession stripping - metricsinroi: transpose scil_volume_stats_in_labels JSON from metric-centric to region-centric so roi column contains GM regions and metric names become column headers (was inverted before) - metricsinroi: rename header column subject_id -> sid - metricsinroi: add outer-key normalization jq (strip leading _, desc-xxx__rest -> rest_xxx) - roivolumes: rename header column subject_id -> sid - roivolumes: fix session prefix stripping in WM bundle names (run substrs loop first to remove subject prefix, then strip session prefix from shorter key) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The scilpy container ships jq 1.5 which does not support elif. Replace the outer-key normalization jq filter with nested if-else-end. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The container's jq (1.5) does not support accessing named capture groups
via field syntax (.d, .rest) inside if-then branches. Replace with
split("__") which is universally supported.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ables - Add exit 1 to the missing-LUT guard so the process fails immediately with a clear message instead of continuing to the scil command - Rename rois/metrics reassignment variables to roi_names/metric_names to avoid shadowing the process input path variables Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d conda deps
Remove the hardcoded session-prefix stripping from WM bundle names (lines
that stripped session_id + "_" from the start of bundle names after
key_substrs_to_remove). This logic is pipeline-specific (depends on how
sf-tractomics names warped files when meta.id excludes the session) and
does not belong in a reusable module. Pipelines that need this behaviour
should add "${meta.session}_" to task.ext.key_substrs_to_remove.
Also populate environment.yml with nibabel>=4.0.0 and numpy>=1.24.0, the
actual runtime dependencies used by the module (previously empty, breaking
conda execution mode).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… test config - Add docker.io/ registry prefix to container declaration - Remove name field from environment.yml (fails nf-core schema validation) - Add ext.prefix to test nextflow.config to prevent null_ in substrs - Add generated nf-test snapshots Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…est config - Add docker.io/ registry prefix to container declaration - Remove name field from environment.yml (nf-core schema disallows it) - Add ext.prefix to test nextflow.config to prevent null_ in substrs_to_remove Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
….yml nf-core schema requires package=version (not >=). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
AlexVCaron
left a comment
AlexVCaron
left a comment
There was a problem hiding this comment.
Very nice work, eager to see it merged into the library ! I left a few comments, don't hesitate to reach out for any question !
| tag "$meta.id" | ||
| label 'process_single' | ||
|
|
||
| container "docker.io/scilus/scilpy:2.2.2_cpu" |
There was a problem hiding this comment.
Since you dabble without Scilpy, let's create a custom lightweight container for this module using Seqera Containers. Here's the built image to use : community.wave.seqera.io/library/nibabel_numpy:4148b92a8cbc6976
There was a problem hiding this comment.
You are right, thank you for the reference. I swapped the dockers and checked the output is identical.
| In WM mode (use_label = false): list of binary or weighted NIfTI mask files, | ||
| one per bundle. In GM mode (use_label = true): single NIfTI label atlas where | ||
| each integer value corresponds to a region defined in the Look-Up Table (LUT). |
There was a problem hiding this comment.
Also add a description of the expected output name (aka how it's extracted from the filename) when this input is a list of rois (what you call WM mode).
There was a problem hiding this comment.
Also, the WM/GM modes nomenclature should be generalized, it's not specific to the single context. It's atlas/labelmap mode versus labels mode. I really like that the module can handle both !
There was a problem hiding this comment.
Also add a description of the expected output name (aka how it's extracted from the filename) when this input is a list of rois (what you call WM mode).
Added. The ROIs input description now spells out the rule for labels mode.
In practice: take the basename, strip the .nii / .nii.gz
extension, then remove each substring listed in key_substrs_to_remove from both the start and the end of what remains.
There's a worked example in the description with
key_substrs_to_remove = ["sub-01_", "_warped"], the file sub-01_AF_L_warped.nii.gz gives the ROI name AF_L.
Also, the WM/GM modes nomenclature should be generalized, it's not specific to the single context. It's atlas/labelmap mode versus labels mode. I really like that the module can handle both !
No problem, i updated the meta.yml description, both inputs, the output, all three args, the inline comments in
main.nf, and the two test case names. GM mode is now atlas/labelmap mode (use_label = true) and WM mode is labels
mode (use_label = false).
Atlas/labelmap mode counts regions inside one image whose integer voxel values encode them (a Desikan parcellation, a FreeSurfer segmentation, any LUT-driven atlas), while labels mode counts one file per ROI across a list of separate masks (tractography bundles, lesion masks, tissue masks). Basically the rename exists because the split is about how many files are handed in, not whether the tissue happens to be grey or white matter.
Also the csv outputs of both modes are unified into a single column under the name 'rois' ( instead of 'regions' or 'bundle' which were likely too specific).
| ch_stats_mean = channel.empty() | ||
| ch_stats_std = channel.empty() | ||
|
|
||
| if (options.run_roi_metrics != false) { |
There was a problem hiding this comment.
What's the reason for the negated check against false ?
There was a problem hiding this comment.
There is no reason. The key is always boolean. However, this check no longer exists since the key disappeared after my modifications for the generalization of the nomenclature.
There was a problem hiding this comment.
I think it would be preferable the subworkflow takes an already registered atlas in inputs. This way, a user can configure its own registration flow the way he wants.
There was a problem hiding this comment.
I think you're right about the fact that the subworkflow currently welds two unrelated jobs together (getting the atlas into subject space, and computing statistics on it). It opens the door to duplicate registration works. However, I am not sure this PR should implement that. @levje is the author and maintainer of atlas_roimetrics, and this is a redesign of his interface rather than a fix to what I'm adding. I think this should be the object of a different PR.
For now, I added the registered atlas as an optional input, namely 'ch_registered_atlas', rather than modifying the whole flow.
The output CSV named its ROI column `bundle` in labels mode and `region` in atlas/labelmap mode, so a consumer had to know which mode produced a file before it could parse it. Both are now `roi`, making the emitted column set identical in every case: sid, session, run, roi, volume_voxels, volume_mm3. BREAKING CHANGE: the ROI column of *_volumes.csv is renamed from bundle/region to roi.
Adds an optional fourth input, ch_registered_atlas, carrying ROIs already in each subject's space as a list of masks, a labelmap with its LUT, or both. It is selected with a new use_registered_atlas option, mutually exclusive with use_atlas_iit. On that path no atlas is downloaded and no registration runs, so callers can bring their own registration flow. Downstream statistics now read two provenance-neutral channels rather than the transform outputs directly, which leaves behaviour under use_atlas_iit unchanged — the four existing snapshots are byte-identical. BREAKING CHANGE: ATLAS_ROIMETRICS takes four inputs instead of three. Callers not supplying a registered atlas must pass channel.empty() at position 2.
…ut axes The option set still named things after tissue rather than after ROI representation, and it merged two independent decisions into one. Replace run_gm_roimetrics, run_roi_metrics and run_roi_volumes with two lists: roi_sources: 'masks' and/or 'labelmap' roi_outputs: 'metrics' and/or 'volumes' Four booleans become two options with full 2x2 selectivity. Each source is warped only when requested, so asking for the labelmap alone no longer warps the bundle masks. The labelmap gate no longer needs to be OR'd past for the pre-registered atlas: both provenances read the same two axes. Rename the process aliases accordingly. The transforms and the registration only ever run under the IIT atlas, so they are scoped to it; the statistics serve both provenances, so they are named after the representation they read. REGISTER_ATLAS_REF -> REGISTER_IIT_REF TRANSFORM_ATLAS_BUNDLES -> TRANSFORM_IIT_BUNDLES TRANSFORM_GM_ATLAS -> TRANSFORM_IIT_GM_ATLAS STATS_METRICSINROI -> STATS_MASK_METRICS STATS_GM_ROIMETRICS -> STATS_LABELMAP_METRICS STATS_WM_VOLUMES -> STATS_MASK_VOLUMES STATS_GM_VOLUMES -> STATS_LABELMAP_VOLUMES BREAKING CHANGE: the emitted channels are renamed to mask_stats_json, mask_stats_tab_mean, mask_stats_tab_std, mask_volumes and their labelmap_* counterparts. Callers must update both their option map and their output references, and any config selector naming the renamed processes.
Three leftovers used tissue words to describe behaviour that is not tissue-specific. atlas_iit's run_gm_roimetrics option computes no ROI metrics -- it decides whether the GM Desikan atlas is fetched at all. Renamed to fetch_gm_desikan, which keeps GM only where it is part of the artefact's name. A comment in stats/metricsinroi still referred to "WM mode", and the first_suffix example in stats/roivolumes used "atlas-iit-gm", tying the module's documentation to one pipeline's tissue naming. Occurrences that name what the IIT atlas actually distributes are left alone: the GM Desikan filenames, the gm_atlas and gm_lut emits, and the atlas_iit_gm_* options. BREAKING CHANGE: the atlas_iit option run_gm_roimetrics is renamed to fetch_gm_desikan.
2 participants
Describe your changes
New module stats/roivolumes that computes voxel counts and volumes (mm³) for ROI regions. Supports two modes: WM (list of binary/TDI masks, one per bundle) and GM (single label atlas with JSON Look-Up Table (LUT)). Uses nibabel and numpy directly — no scilpy CLI dependency.
List test packages used by your module
tractometry.zip (WM binary masks test)
plot.zip (GM label atlas test from Brainnetome atlas)
Checklist before requesting a review
./modules/nf-neuro/<category>/<tool>/main.nf./modules/nf-neuro/<category>/<tool>/meta.yml./modules/nf-neuro/<category>/<tool>/environment.yml./modules/nf-neuro/<category>/<tool>/tests/main.nf.testmain.nf.test.snapsnapshotsprettierandeditorconfig-checkerto fix common syntax issuesnf-core modules lintand fix all errors [1][1] Note: main_nf_ext_key and main_nf_meta_key failures remain. To my understanding, these are nf-core v4.0.3-only checks not enforced by the nf-neuro CI (which uses ≤3.5.2). The same patterns (ext.first_suffix, ext.use_label, ext.key_substrs_to_remove, meta.session, meta.run) exist in the already-merged stats/metricsinroi module. Please advise if a repo-wide fix is expected
Thank you for your time for reviewing this contribution!