Add TORTOISE DIFFPREP as a head-motion-correction backend - #1066
Merged
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1066 +/- ##
==========================================
+ Coverage 47.52% 50.19% +2.66%
==========================================
Files 66 68 +2
Lines 9918 10565 +647
Branches 1101 1180 +79
==========================================
+ Hits 4714 5303 +589
- Misses 4970 5000 +30
- Partials 234 262 +28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add diffprep_motion, diffprep_quadratic, diffprep_cubic to --hmc-model choices - Add --diffprep-config CLI argument mirroring --eddy-config - Add diffprep_config to execution._paths and workflow class in config.py - Add validate_diffprep_config() in utils/misc.py - Add default qsiprep/data/diffprep_params.json - Add TDD tests: parser choices + validate_diffprep_config unit tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements _TORTOISEProcessInputSpec, _TORTOISEProcessOutputSpec, and TORTOISEProcess (wrapping the TORTOISEProcess binary in DIFFPREP mode) in qsiprep/interfaces/tortoise.py. Adds test file qsiprep/tests/test_interfaces_diffprep.py with cmdline construction test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eview) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extracts the rigid 6-DOF (3 translations mm, 3 rotations rad) from the per-volume DIFFPREP transforms file produced by TORTOISEProcess, writing them to a plain-text motion file for downstream confound reporting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…TORTOISE b-matrix Implements the inverse of make_bmat_file: reads a TORTOISE .bmtxt (6-column per-volume b-matrix) and recovers b-values via trace(B) and gradient directions via principal eigenvector of B. Adds round-trip test confirming b-value and direction recovery from hand-crafted b-matrix entries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… 4 review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Task 5) Create qsiprep/workflows/dwi/diffprep.py with init_diffprep_hmc_wf: a bake-in HMC-only DIFFPREP workflow that is a drop-in peer of init_fsl_hmc_wf. Add generate_diffprep_boilerplate to tortoise.py and the workflow-construction test to test_interfaces_diffprep.py. Fixes the ExtractB0s bval_file gap noted in the brief by wiring bmat_to_fsl.bval_file -> extract_b0s.bval_file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… 5 review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add import for init_diffprep_hmc_wf, module-level _diffprep_order() helper, and elif diffprep_ dispatch in init_dwi_preproc_wf HMC block. Add test_base_selects_diffprep pinning the helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t (Task 6 review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Document the three new --hmc-model values (diffprep_motion, diffprep_quadratic, diffprep_cubic) and --diffprep-config flag in the "Head motion correction model" section of docs/quickstart.rst, covering their use case for non-shelled / CS-DSI acquisitions where FSL eddy cannot be used and SHORELine does not correct eddy currents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y polarity, orientation, PATH guard (final review) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CNR --- DIFFPREP emitted a zeros placeholder for cnr_map, which get_cnr_values() publishes into the image QC TSV as CNR_mean/median/stdev -- so every run reported CNR_mean = 0.0, reading as catastrophically bad data. The workflow already runs a MAPMRI fit (SynthesizeDWIs) for the carpet plot, and it emits exactly the three inputs SHORELine's CalculateCNR needs. Reuse it, so the map means the same thing it does for the other backends: per-voxel var(predicted) / var(predicted - observed). No new model fit, and no new failure mode -- synth_dwis already gated slice_quality. The derivative label was also invalid BIDS: model= came straight from hmc_model, and "diffprep_quadratic" contains the entity separator, so model-diffprep_quadratic_stat-cnr_dwimap cannot be parsed back. The model entity names the signal model the CNR came from (it only looked like the HMC backend because those strings coincide for 3dSHORE/eddy/ tensor), so the diffprep backends now report model-MAPMRI. Every other backend maps to itself; no existing filename changes. The MAPMRI predictions are in-sample, unlike SHORELine's, which excludes q-space neighbours of the volume being predicted. Values are therefore optimistically biased and not comparable across backends; the sidecar Description says so rather than shipping a flattering number silently. --sloppy -------- The DIFFPREP node ignored --sloppy, unlike its peers, and burned the full 1h no-output timeout in CI. Every DWI is registered to the b=0 regardless; the cost is the second pass (fit DTI+MAPMRI, synthesize a contrast-matched target per volume, re-register). --niter 0 disables that outright, but only bites on high-b data, so sloppy runs also drop to rigid (-c motion) to bound the always-run first pass. Both are sloppy-only and logged loudly, since eddy correction is then off. is_human_brain is deliberately left alone: it reaches the same iterative gate but is not a speed knob -- it makes auto-masking read a <stem>_noise.nii and changes structural-target masking on T2Wreg. DRBUDDI already collapses its diffeomorphic schedule under --sloppy, but that has no effect on Step1, where its remaining time goes. Skip Step1's rigid+diffeo+rigid loop as well. --DRBUDDI_disable_initial_rigid is the bigger lever but is not used: it suppresses bdown_to_bup_rigid_trans_h5, which DRBUDDIAggregateOutputs dereferences unguarded on the rpe_series FA branch. Tests ----- Add the diffprep integration manifest, derived from the outputs of the CI run itself. diffprep_drbuddi stays on check_outputs=False until it produces a complete run.
=== Do not change lines below ===
{
"chain": [],
"cmd": "bash -lc 'if pixi lock --check; then echo '\"'\"'Lockfile up to date'\"'\"'; else pixi lock; fi'",
"exit": 0,
"extra_inputs": [],
"inputs": [
"pixi.lock",
"pyproject.toml"
],
"outputs": [
"pixi.lock"
],
"pwd": "."
}
^^^ Do not change lines above ^^^
ruff 0.16.0 was released and `pipx run ruff` follows latest, so the style job went red on 64 errors across 48 files without a code change. The new failures are all rule families this project never selected (TRY, RUF, SIM, FURB, S320), in code untouched by any recent commit. Pin the CI ruff, mirroring PennLINC/qsirecon#394, and align the pre-commit rev with it so the "keep in sync" comment is true. 0.15.21 was chosen because it is verified to pass `ruff check .`, `ruff format --diff .` and the separate ISC001 hook on this tree, so aligning pre-commit from v0.6.2 causes no reformatting churn. Also fix "unparseable" -> "could not be parsed back" in a test docstring. Note the codespell action runs with no options, so it does not read [tool.codespell] in pyproject.toml.
For rpe_series fieldmaps, qsiprep concatenates the opposing-PE volumes into one 4D file for FSL eddy (preprocess_rpe_series). DIFFPREP models a single phase axis per run -- TORTOISE itself runs DIFFPREP once per PE direction -- so feeding it the concatenation does not error, it silently mis-corrects half the volumes (and, before this, ran past the 1h CI timeout doing so). Raise a clear NotImplementedError for diffprep_* + rpe_series pointing to an epi fieldmap or eddy/3dSHORE, rather than returning a wrong answer. The epi path is unaffected: an epi fieldmap does not trigger the concatenation, so DIFFPREP gets a clean single-PE series and DRBUDDI gets blip-up/down b=0s. Point the diffprep_drbuddi integration test at the drbuddi_epi dataset (the path that works) and add a unit test asserting rpe_series is rejected. Full rpe_series support -- per-direction DIFFPREP plus a predicted single-shell series so DRBUDDI can derive usable b0/FA on non-shelled CS-DSI -- is a planned follow-up.
…FPREP) DIFFPREP models a single phase axis and a single b=0 reference for a whole file (TORTOISEProcess runs DIFFPREP once per PE direction). qsiprep merges the two opposing-PE series into one 4D file for FSL eddy, and feeding that merge to one DIFFPREP run silently mis-corrects. This adds the per-direction path the guard was standing in for, for the shelled case: - SplitDWIsByDistortionGroup re-splits the already-merged (denoised + b0-harmonized) series back into its two PE groups, keeping pre_hmc's per-direction denoising and cross-direction b0 harmonization intact. - DIFFPREP is run once per group (each single-PE, --epi off), mirroring TORTOISE's own for(PE=0;PE<2) loop. A low-b0-count warning surfaces the case where select_best_b0 would degrade to a single reference. - ConcatenateDIFFPREPGroups recombines the two corrected series in the original merged order (order-preserving, handles interleaved groups) into a single corrected DWI / bmtxt / transforms triple -- a drop-in for one DIFFPREP node, so every downstream node (split, QC, CNR, motion params) is unchanged. - The recombined flat list flows into the stock init_drbuddi_wf rpe_series path, which re-splits up/down itself; DRBUDDI needs no changes. A data-based detector (_rpe_series_is_shelled, overridable via --diffprep-config "rpe_series_shelled") gates this: shelled reverse-PE series run the path above; non-shelled (CS-DSI) series still raise, pending the predicted-shell synthesis follow-up. The whole change is confined to the diffprep path -- pre_hmc, eddy/fsl and DRBUDDI are untouched. Adds unit tests for the split/recombine interfaces, the detector, and the rpe_series workflow wiring, plus a diffprep_rpe_series integration marker/test on the (shelled) tinytensor_rpe_series dataset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DRBUDDI's rpe_series path fits its own [b0, FA] per phase-encoding direction and uses it as a 2-channel registration target. On a q-space grid (CS-DSI) that tensor fit is ill-conditioned, so DRBUDDI would silently produce a poor A0/FA. This replaces the Tier-1 non-shelled guard with the validated synthesize-a-shell approach from the csdsi-preproc offshoot: - Per phase-encoding direction, synthesize a tensor-fittable [b0 + 32*b1000] shell: 3dSHORE SignalPrediction (reused from qsiprep) on the already-corrected volumes at a deterministic evenly-distributed direction set, with the side's own measured b0 mean as volume 0. Adapted from the offshoot's init_predict_shell_wf, minus its HMC ApplyTransforms step (DIFFPREP already baked correction in). - Stage both shells (StageDRBUDDIPair: distinct stems, uncompressed float32) and run the DRBUDDI interface directly in rpe_series mode; it derives its own consistent A0/FA from the single-shell inputs. - The deformation DRBUDDI estimates is applied to the *real* corrected data via the STOCK DRBUDDIAggregateOutputs (unchanged), keyed by the real series' per-volume blip_assignments. The synthesized shell of a side shares that side's distorted geometry, so its deformation is valid for the real volumes. init_drbuddi_wf is deliberately not reused here: its GatherDRBUDDIInputs rebuilds DRBUDDI's registration images from the real (non-shelled) series, which is exactly what must be avoided. Small pure-Python interfaces are ported from the offshoot (MergeVolumes4D, WriteFSLGradFiles, WriteBmatTORTOISE, StageDRBUDDIPair, WriteDRBUDDIJSON, equally_distributed_directions) plus a SplitCorrectedByGroup helper; all live in interfaces/tortoise.py, and DRBUDDI itself is untouched. Shelled vs non-shelled is auto-detected (_rpe_series_is_shelled), overridable via --diffprep-config "rpe_series_shelled". The synthesis path emits a loud EXPERIMENTAL warning: it has no CS-DSI CI dataset and must be validated against real data (its failure mode is a silent wrong answer). Adds pure-Python tests for every ported interface, the split helper, and the non-shelled workflow wiring. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Validated against a real CS-DSI HASC55 acquisition (sub-0123): the previous heuristic misclassified it as shelled. Two problems, both fixed: - It pooled the AP and PA b-values, doubling every shell's population; HASC55 has 4 volumes near b=1195 per direction, which pooled to 8 and tripped the min_shell_dirs=6 low-b test. Now each phase-encoding direction is evaluated independently and both must be shelled. - A low-b population count alone can't tell a q-space grid from a real shell (a grid can pack samples near a low-b radius). Add a grid guard on the number of distinct b-value shells: HASC55 fragments into ~18 shells, DTI has 1, multi-shell HARDI a handful (cap: max_shells=7). This is the decisive test. Adds the real HASC55 scheme as a regression fixture. min_shell_dirs stays 6 (the tensor-fit floor) so a minimal-direction DTI still uses the stock path rather than the also-marginal synthesis path; the grid guard does the CS-DSI rejection.
Adds the wiring for a downsampled CS-DSI HASC55 reverse-PE-series fixture that exercises the Tier-2 predicted-shell DRBUDDI path (the shelled tinytensor fixture only covers Tier 1). The fixture (sub-2345/ses-1: AP+PA HASC55 + T1w/T2w, downsampled to ~7 mm DWI / ~4 mm anat, 8 MB) groups cleanly into one rpe_series scan-group and is classified non-shelled by the detector. The archive isn't uploaded yet, so the download URL is a placeholder and the test skips cleanly until the data lands (guarded against download_test_data's create-dir-before-fetch quirk). Maintainer steps: upload csdsi_rpe_series.tar.xz to Box, replace the placeholder URL in tests/utils.py.
Replaces the placeholder URL for the non-shelled CS-DSI HASC55 reverse-PE
fixture with the real Box link. Verified: download_test_data('csdsi_rpe_series')
fetches + extracts to the csdsi_hasc55/ BIDS root, which groups into one
rpe_series scan-group and reads non-shelled (Tier-2 synthesis path).
The diffprep_rpe_series (shelled, tinytensor) and diffprep_csdsi_rpe_series (non-shelled CS-DSI, predicted-shell synthesis) tests existed but no CI job invoked them, so the new csdsi_rpe_series data was never consumed. This: - adds the csdsi_rpe_series download (URL in data_versions.txt, which also invalidates the data cache so get_data re-fetches it), - adds both markers to the integration_test matrix and merge_coverage requires. diffprep_rpe_series reuses the already-downloaded drbuddi_rpe_series data. Heads-up: diffprep_csdsi_rpe_series exercises the experimental predicted-shell path end to end for the first time, so it may surface issues on its first CI run (that is the point). Skip an individual job with [skip <marker>] in a commit message if needed.
t2w_sdc was set to bool(subject_data.get('t2w')) regardless of --anat-modality.
With --anat-modality none there is no anatomical workflow, so t2w_unfatsat is
never produced -- yet a T2w in the dataset still switched on the DRBUDDI
multimodal / TORTOISE T2Wreg / extended-pepolar-report T2w paths. The report's
t2w_n4 node then ran with an empty input_image and the job failed (and hung).
No prior DRBUDDI test shipped a T2w, so nothing exercised this; the new
csdsi_rpe_series fixture (which ships a defaced T2w) is the first, and its
diffprep_csdsi_rpe_series CI job surfaced it -- after the predicted-shell
synthesis + DRBUDDI + aggregate all completed successfully. Gate t2w_sdc on
anatomical processing actually being enabled.
The T2w-multimodal DRBUDDI branch itself is unchanged and still active when a
T2w is present AND anat processing runs (--anat-modality t1w/t2w).
Cover the T2w handling the diffprep_csdsi_rpe_series CI failure exposed, at the graph level -- import the subworkflows and assert the connections, no TORTOISE binaries and no resampling: - extract _t2w_available_for_sdc(subject_data) and test the gate: a T2w only drives SDC when anatomical processing runs (T2w + --anat-modality none -> off), which is the fix for the empty-input crash. - test the extended pepolar report: with a T2w, t2w_n4.input_image is wired from inputnode.t2w_image (the node that failed); without one it uses the t1w-seg branch and builds no t2w_n4. - test the predicted-shell DRBUDDI path: with T2w SDC on, the T2w is fed to DRBUDDI as its structural and DRBUDDI's used structural flows out as t2w_image; aggregate_drbuddi takes the structural only from drbuddi (a second source would double-connect and raise at build); with T2w SDC off, DRBUDDI gets no structural.
The flag only exists on a patched TORTOISE, so a stock binary must never see it. 2.5mm is a deliberate speed choice for smoke tests, not a validated registration resolution.
T2Wreg performs real susceptibility distortion correction but carries no fieldmap, so with fieldmap_type=None it fell through both reportlet gates in init_dwi_preproc_wf and silently produced no figure -- despite pre_sdc_template, b0_template, t1_seg and itk_b0_to_t1 all being computed and plumbed already. This also removes an asymmetry: the identical correction on a group tagged 'syn' did get a reportlet, purely because 'syn' is absent from the gate's exclusion tuple. Uses desc-sdcT2w with its own reports-spec entry; a desc missing from the spec is written to disk but never shown in the HTML report.
Non-shelled reverse-PE series auto-triggered a qsiprep-side synthesis workflow
on the theory that DRBUDDI cannot tensor-fit a usable [b0, FA] from a q-space
grid. Measurement contradicts that premise: on real HASC55 the plain-tensor FA
resolves corpus callosum, internal capsule and corona radiata, and drives
DRBUDDI to within ~0.002 correlation of a synthesized-shell target for roughly
half the runtime. Across 9 sessions and two independent metrics the synthesis
benefit is at or near zero.
So non-shelled series now take the same stock DRBUDDI path as everything else,
and shell synthesis is exposed as an opt-in that hands the work to TORTOISE
itself (--DRBUDDI_synth_shell_bval), which does it per phase-encoding direction
inside CreateCorrectionImage for one interpolation instead of a separate
qsiprep-side resample:
--diffprep-config '{"drbuddi_synth_shell_bval": 1000}'
Absence of the key emits no flag at all, so a stock (unpatched) TORTOISE is
unaffected. Detection still runs and now points users at the opt-in when a
non-shelled series is seen.
Removes 149 lines of workflow plus 4 now-dead imports.
The pinned qsiprep-drbuddi:26.1.3 is a stock-parser build: it rejects --epi_working_res, which --sloppy emits on every CI integration run, and DRBUDDI answers an unrecognised parameter by printing "Unknown command line parameter" and then exiting 0. nipype reads that as success and the run dies afterwards collecting a bdown_to_bup_rigidtrans.hdf5 that was never written -- which is why all six DRBUDDI-flavoured jobs failed while every other job passed. 26.8.0 is built from PennLINC/qsiprep_TORTOISE upstream-port and accepts --epi_working_res and --DRBUDDI_synth_shell_bval/_ndirs. ARG BASE_IMAGE has to move too. image_prep only builds Dockerfile.base when the tag named in Dockerfile is absent from Docker Hub, so bumping TAG_TORTOISE alone would have been silently ignored and the old base pulled unchanged.
--DRBUDDI_start_with_diffeomorphic_for_rigid_reg was set under --sloppy to cheapen Step1's rigid+diffeo+rigid loop. It never did: TORTOISE has that option commented out of DRBUDDI_parserBase.cxx along with its getter, so it cannot reach the registration at all. Worse, whether it is merely useless or fatal depends on the build. The stock 26.1.3 parser swallowed a valueless unknown flag silently; the 26.8.0 fork build rejects it, printing "Unknown command line parameter" and exiting 0 -- the same trap that --epi_working_res fell into, so upgrading TORTOISE alone would have left CI red for a second reason. Verified by running DRBUDDI standalone on tinytensor_epi: with the flag, instant exit 0 and no outputs; without it, exit 0 in 41s, "Using requested EPI working resolution 2.5 mm", and all ten expected outputs including the rigid transform hdf5. --DRBUDDI_disable_initial_rigid is disabled in the parser the same way, and would additionally suppress bdown_to_bup_rigid_trans_h5, which DRBUDDIAggregateOutputs dereferences unguarded on the rpe_series FA branch. Neither flag is safe to send, so the test that asserted the first one was emitted becomes a guard that neither ever is, at both the interface and the workflow level. Also fixes a mangled import block, and carries the ruff-format changes this file already needed.
fcd33f1 pinned ruff in lint.yml and aligned the pre-commit rev, but left the dev extra at "ruff ~= 0.4.3". That resolves to 0.4.x, eleven minor releases behind CI's 0.15.21, so anyone who installs the project's own tooling lints against different rules than the job that gates the PR. The two disagree in both directions, not just in strictness: 0.4.10 reports 45 errors on this tree and 0.15.21 reports 7, and PT001 is inverted between them (0.4.x wants @pytest.fixture(), 0.15.x wants @pytest.fixture). "Fixing" lint with the dev-extra ruff therefore introduces CI failures. Pin it exactly, and note the two files it has to stay in step with. Also applies what the pinned ruff actually asks for: four imports in diffprep.py left unused when DRBUDDI moved to fieldmap/drbuddi.py, the PT001 fixture decorator, and two format-only line joins.
=== Do not change lines below ===
{
"chain": [],
"cmd": "bash -lc 'if pixi lock --check; then echo '\"'\"'Lockfile up to date'\"'\"'; else pixi lock; fi'",
"exit": 0,
"extra_inputs": [],
"inputs": [
"pixi.lock",
"pyproject.toml"
],
"outputs": [
"pixi.lock"
],
"pwd": "."
}
^^^ Do not change lines above ^^^
mattcieslak
force-pushed
the
tortoiseproc
branch
from
August 10, 2026 14:34
d6b72b3 to
2cdbd9f
Compare
=== Do not change lines below ===
{
"chain": [],
"cmd": "bash -lc 'if pixi lock --check; then echo '\"'\"'Lockfile up to date'\"'\"'; else pixi lock; fi'",
"exit": 0,
"extra_inputs": [],
"inputs": [
"pixi.lock",
"pyproject.toml"
],
"outputs": [
"pixi.lock"
],
"pwd": "."
}
^^^ Do not change lines above ^^^
mattcieslak
marked this pull request as ready for review
August 10, 2026 14:51
The qsiprep-side shell synthesis was dropped in favor of the stock DRBUDDI tensor fit (with TORTOISE-side synthesis as the opt-in), but the workflow builder, five interfaces, and their tests stayed behind with no callers. Also drop the process planning documents from docs/ -- they are not documentation.
The eddy and DIFFPREP backends pass gpu_enabled('drbuddi') into
init_drbuddi_wf, but the SHORELine/hmc_sdc call site did not, so
--gpu drbuddi validated the GPU at parse time and then silently ran the
CPU build.
Drop measurement anecdotes, change-history narration, section banners, and the Sphinx #: idiom; keep only the notes that explain genuinely counterintuitive behavior. Also merge drbuddi.py's duplicate import.
The docs still described the removed auto-synthesis path; non-shelled reverse-PE series use the stock DRBUDDI tensor fit, with TORTOISE-side shell synthesis available through "drbuddi_synth_shell_bval".
_load_diffprep_config no longer defaults use_cuda, so absence stays observable and gpu_enabled() cannot mistake a shipped default for user intent; the test still asserted the old defaulting behavior.
tsalo
commented
Aug 11, 2026
tsalo
left a comment
tsalo
left a comment
Member
Author
There was a problem hiding this comment.
Just a few minor thoughts
Comment on lines
+83
to
+86
| # Keep this pin in sync with the ruff pin in .github/workflows/lint.yml and | ||
| # the rev in .pre-commit-config.yaml. A looser spec here installs a ruff | ||
| # that disagrees with CI -- 0.4.x and 0.15.x report inverted PT001, so | ||
| # "fixing" lint locally introduced CI failures. |
Member
Author
There was a problem hiding this comment.
Suggested change
| # Keep this pin in sync with the ruff pin in .github/workflows/lint.yml and | |
| # the rev in .pre-commit-config.yaml. A looser spec here installs a ruff | |
| # that disagrees with CI -- 0.4.x and 0.15.x report inverted PT001, so | |
| # "fixing" lint locally introduced CI failures. | |
| # Keep this pin in sync with the ruff pin in .github/workflows/lint.yml and | |
| # the rev in .pre-commit-config.yaml. |
| MULTISHELL_OUTPUT_URL=https://upenn.box.com/shared/static/hr7xnxicbx9iqndv1yl35bhtd61fpalp.xz | ||
| SINGLESHELL_OUTPUT_URL=https://upenn.box.com/shared/static/9jhf0eo3ml6ojrlxlz6lej09ny12efgg.gz | ||
| DRBUDDI_RPE_SERIES_URL=https://upenn.box.com/shared/static/j5mxts5wu0em1toafmrlzdndves1jnfv.xz | ||
| CSDSI_RPE_SERIES_URL=https://upenn.box.com/shared/static/3mmagbtddgb4lpmlc5vs4jnsyf1etp3d.xz |
Member
Author
There was a problem hiding this comment.
If you use PAFIN after removing the part-mag bval and bvec files (i.e., just keeping the copies without a part entity), that'll also test #1080. Plus the phase data will be useful for testing complex dwidenoise/dwidenoise2.
Comment on lines
+27
to
+29
| # Keep this pin in sync with the ruff rev in .pre-commit-config.yaml | ||
| # and the dev pin in pyproject.toml. Unpinned, CI silently follows new | ||
| # ruff releases and goes red without a code change. |
Member
Author
There was a problem hiding this comment.
Suggested change
| # Keep this pin in sync with the ruff rev in .pre-commit-config.yaml | |
| # and the dev pin in pyproject.toml. Unpinned, CI silently follows new | |
| # ruff releases and goes red without a code change. | |
| # Keep this pin in sync with the ruff rev in .pre-commit-config.yaml | |
| # and the dev pin in pyproject.toml. |
This was referenced Aug 12, 2026
Closed
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
4 participants
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
Adds TORTOISE v4 DIFFPREP as a first-class
--hmc-modeloption, alongsideeddy,3dSHORE, andtensor. DIFFPREP fits a signal model over arbitrary q-space, so it corrects head motion and eddy currents on non-shelled data (e.g. compressed-sensing DSI) — the gap where FSL eddy can't run (no shells) and SHORELine corrects motion but not eddy currents.Three variants, selected by suffix:
--hmc-modeldiffprep_motiondiffprep_quadraticdiffprep_cubicScope
This backend is HMC-only (motion + eddy current). It follows the same "bake-in" model as eddy: TORTOISE's resampled DWI and rotated gradients become the workflow outputs, with identity per-volume affines.
It does not perform susceptibility distortion correction. If a fieldmap is present for a DWI series, the run fails fast with a clear
NotImplementedErrorrather than silently skipping SDC — use--hmc-model eddy/3dSHORE, or--ignore fieldmaps.Changes
--hmc-modelchoices:diffprep_motion,diffprep_quadratic,diffprep_cubic.--diffprep-configflag (mirrors--eddy-config), backed byconfig.workflow.diffprep_config,validate_diffprep_config, and a defaultqsiprep/data/diffprep_params.json.qsiprep/interfaces/tortoise.py)TORTOISEProcess— DIFFPREP runner.DIFFPREPMotionParams— per-volume rigid motion parameters for confounds / QC.BmatToFSLGradients— recovers FSL bval/bvec from the TORTOISE b-matrix.generate_diffprep_boilerplate— methods boilerplate.qsiprep/workflows/dwi/diffprep.py)init_diffprep_hmc_wf— a drop-in peer ofinit_fsl_hmc_wfwith an identical inputnode/outputnode contract, all in LPS+.ConformDwiso they stay consistent with the reoriented image.qsiprep/workflows/dwi/base.py)init_dwi_preproc_wfbranches to the new workflow whenhmc_model.startswith('diffprep_').--shoreline-itersguard are untouched (fully backward compatible).docs/quickstart.rst)Diff:
+688 / −3across 10 files (parser, config, misc, data, interfaces, workflow, base, docs, plus tests).Testing
10 new tests pass:
init_diffprep_hmc_wf.Tests are intentionally offline (the TORTOISE binaries aren't available in CI), so the interface tests assert command-line structure rather than exact flag strings.
Not yet validated against the real binary
These TORTOISE-binary-dependent details still need reconciling against
TORTOISEProcessinside the qsiprep container before a production run. They are deliberately isolated so they don't affect the public contract:TORTOISEProcessargument flag names and its output filenames.DIFFPREPMotionParamsreads the first 6 columns)..bmtxtcolumn order and b-value scale (BmatToFSLGradients— the round-trip test is internally consistent but not checked againstFSLBVecsToTORTOISEBmatrix).antsApplyTransformsaccepts the identity.matfiles emitted forto_dwi_ref_affines.The TORTOISE binaries themselves already ship in the qsiprep image (copied from
pennlinc/qsiprep-drbuddi), so no Dockerfile change is needed.