Functional outputs & regressor reuse (Stage 5 of #301)#315
Merged
Conversation
regressor_file now carries the raw (unfiltered) regressor .1D files as computed from native-space BOLD. The new bpf_regressor_file field holds the bandpass-filtered version that 3dTproject actually applied, exported with desc-<strategy>Filtered for provenance. This lets the longitudinal pipeline reuse the raw regressors without recomputation, matching the cross-sectional principle: one regressor computation per run, applied in each target space. Port of the regressor-split idea from #282. Co-authored-by: Janhavi Pillai <janhavi.pillai@gmail.com>
FunctionalLongOutputs gains regressed_bold and cleaned_bold dicts keyed by regressor strategy. longitudinal_process now accepts regressor_files (raw .1D from cross-sectional) and re-runs apply_regression + apply_regression_bandpass on the warped BOLD. No regressor recomputation -- same matrix, different target space. bold_mask is now mandatory (was Path | None). BIDS export writes per-regressor desc-regressed and desc-preproc BOLD with reg-<strategy> entity. resolve_longitudinal_func resolves the raw regressor files per strategy. CLI gains --regressor with the same choices as cross-sectional (36-parameter, aCompCor). Port of the longitudinal regression logic from #282. Co-authored-by: Janhavi Pillai <janhavi.pillai@gmail.com>
Unit tests: resolve_longitudinal_func with single/multiple regressors, missing regressor raises, mandatory bold_mask; export file counts and reg-<strategy> entity validation. Tier-2 integration: apply_regression and apply_regression_bandpass on warped BOLD with raw regressors produce non-degenerate outputs. Tier-4 full_pipeline: longitudinal functional outputs exist, regressed and cleaned BOLD have non-zero variance, bold mask is binary.
… integration - Drop regressor_set param from longitudinal_process; iterate regressor_files.keys() instead. The resolve layer already filters to the requested strategies, so the dict keys ARE the set. Eliminates the caller sync burden. - Delete tests/full_pipeline/longitudinal/ -- the conftest hacked the cross-sectional anat brain as a "longitudinal template", which exercises the code path but not the actual transform chain. - Rewrite tests/integration/longitudinal/test_regression_reuse.py as a proper end-to-end CLI test: rbc functional -> rbc longitudinal functional on ds000114 sub-01 ses-test via subprocess. Asserts expected BIDS tree, non-degenerate variance, and binary mask. - Add ds000114_func_derivatives and longitudinal_func_output fixtures to the integration conftest so the full cross-sectional -> longitudinal chain runs on real multi-session data with docker.
fba822b to
a3e7707
Compare
…ssue ds000114_func_derivatives now depends on ds000114_anat_derivatives directly instead of longitudinal_template_output. This ensures rbc functional runs before the template step writes ses-longitudinal files into the derivatives dir, avoiding a potential bids2table discovery issue where the ses-longitudinal anat files interfere with the functional pipeline's anat resolution. longitudinal_func_output depends on both ds000114_func_derivatives and longitudinal_template_output to ensure both cross-sectional functional outputs and the longitudinal template are present before rbc longitudinal functional runs.
Filters.apply() applies the --task filter to ALL rows including anat. Anat files have task=null, so --task fingerfootlips drops them, causing resolve_functional to fail with FileNotFoundError on desc-brain_T1w. Drop --task from both rbc functional and rbc longitudinal functional fixture calls. ds000114 only has one task per session so the filter isn't needed. Remove debug logging from previous commits.
The --task filter applied pl.col("task") == value to all rows, but
anat/dwi files have task=null, so they were silently dropped. This
caused rbc functional --task <label> to fail with FileNotFoundError
on desc-brain_T1w for any dataset.
Fix: use pl.col("task").is_null() | (pl.col("task") == value) so
rows without a task entity (anat, dwi, fmap) pass through.
Affects functional and all orchestration; metrics/qc pre-filter to
datatype=func so they were never hit.
Restore --task in integration fixtures now that the fix is in place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FunctionalOutputs.regressor_fileintoregressor_file(raw, unfiltered) andbpf_regressor_file(bandpass-filtered, export only), so the longitudinal pipeline can reuse raw regressors without recomputationFunctionalLongOutputsgainsregressed_boldandcleaned_bolddicts keyed by regressor strategy;bold_maskis now mandatorylongitudinal_processacceptsregressor_files: dict[str, Path]and iterates its keys to re-runapply_regression+apply_regression_bandpasson the warped BOLD (no redundantregressor_setparameter; the resolve layer filters to the requested strategies)bids/longitudinal/functional.pyexports per-regressordesc-regressedanddesc-preprocBOLD withreg-<strategy>entityrbc longitudinal functionalgains--regressorwith the same choices as cross-sectional (36-parameter,aCompCor)Filters.apply()task filter (--task) was dropping anat/dwi/fmap rows becausetask=nullfor non-functional files fails an equality check. Fixed topl.col("task").is_null() | (pl.col("task") == value). Affectsrbc functional --taskandrbc all --taskglobally, not just longitudinal.Closes the Stage 5 checkbox on #301. Ports work from #282.
Test plan
resolve_longitudinal_funcwith single/multiple regressors (7 tests)export_longitudinal_funcfile counts and entity validation (3 tests)bpf_regressor_filefield and mandatorybold_mask--regressorround-trip throughFunctionalLongArgsFilters.apply()task filter unit tests updated: anat rows preserved, combined filter count correctedrbc functionalthenrbc longitudinal functionalend-to-end on ds000114 sub-01 ses-test via subprocess with docker; asserts expected BIDS tree, non-degenerate variance, binary mask