Skip to content

Anat biascorr masks - #1078

Merged
mattcieslak merged 36 commits into
tortoiseprocfrom
anat-biascorr-masks
Aug 11, 2026
Merged

Anat biascorr masks#1078
mattcieslak merged 36 commits into
tortoiseprocfrom
anat-biascorr-masks

Conversation

@mattcieslak

Copy link
Copy Markdown
Collaborator

I've been running the TORTOISE version of qsiprep on a bunch of quality of life issues that are not really related to TORTOISE.

Most importantly, I figured out that the issue seen in #932 is not due to the data already being prescan normalized, but because the mask is off and small values were causing the log transform to blow up. Niworkflows already solved that problem, so we adopt the same here.

  • There are a couple BIDS outputs that are now going to reasonable places in the outputs, all relating to intramodal templates.
    • I added a datasink for the intramodal template in the anat directory so you can overlay it on the other anats.
  • Since we removed mri_robust_template for creating the unbiased anatomical reference, we switched to using antsRegistration. The settings for that registration were based on SHORELine and were total overkill for anatomical images, resulting in a far too strict convergence threshold and causing all the iterations to always run.
    • anatomical unbiased template construction also benefits from an initial COM alignment that wasn't in the SHORELine settings.
  • This PR adds Rigid and Affine options for the intramodal template. They were available as CLI options but never actually worked. We never got bug reports about this, so it must not have been widely used.
  • There were some new flags I added to TORTOISE CLIs that help with multiprocessing.
  • I added a "TSNR" for the unbiased anatomical templates and the intramodal template. These images are pretty useful for making sure registration worked well.
    • There's also a tabular qc file for the individual files that go into the template. This has been helpful for identifying bad sessions

Three related defects:

1. No way to skip N4 on anatomicals. --b1-biascorrect-stage is consumed only by
   the DWI workflows (merge.py, pre_hmc.py, finalize.py) and never reaches the
   anatomical path, where n4_correct ran unconditionally. Console-normalized
   data got N4'd regardless, which can add artifacts rather than remove them.

   Adds --anat-biascorrect {n4,auto,none}. 'auto' reads the BIDS ImageType field
   and skips N4 when every anatomical is marked NORM. A mixed set still gets N4,
   since it cannot be merged consistently otherwise, and missing metadata is
   treated as un-normalized -- running N4 needlessly is milder than skipping it
   when it was needed. Default stays 'n4' to preserve current behaviour.

   The decision is resolved in workflows/base.py, the only scope holding the
   actual file paths, and separately per contrast so a normalized T1w does not
   decide the T2w's fate.

2. The template merge registered whole heads. A rigid fit is then driven partly
   by face, jaw and neck -- structures that move relative to the brain when head
   placement changes -- so non-brain tissue could pull the brains out of
   alignment and blur the template.

   init_b0_hmc_wf/linear_alignment_workflow gain an optional template_mask.
   Only one mask is needed: the fixed image in every registration is the
   template, so a single mask in template space covers all of them. It is built
   with SynthStrip on the first conformed image and dilated to leave slack for
   the between-image motion the registration exists to remove. Masking is opt-in
   at the workflow level, so the DWI b=0 callers are unaffected.

3. Add-on modalities inherited both, since init_t2w_preproc_wf shares
   init_anat_template_wf with T1w. Both fixes therefore reach the T2w that
   drives DRBUDDI and TORTOISE T2Wreg.

Note 'bias_corrected' is a port name, not a promise: with N4 disabled it carries
the conformed image, since ACPC normalization and the skull-on derivative both
read that port.
--intramodal-template-transform and --intramodal-template-iters were never
passed to init_intramodal_template_wf. config.workflow.intramodal_template_transform
appeared exactly once in the codebase -- its own definition -- so every intramodal
template was BSplineSyN with 2 iterations no matter what was requested. Anyone
choosing a linear transform to avoid nonlinearly warping genuine between-session
differences got precisely the thing they were avoiding, silently.

'Rigid' is not in antsMultivariateTemplateConstruction2's enum
(BSplineSyN/SyN/Affine), so the CLI advertised a choice the backend could never
honour. Linear templates are now built with init_b0_hmc_wf, which supports both
Rigid and Affine; nonlinear modes still use mvtc2, now actually receiving the
requested transform and iteration count.

Linear templates initialize by centre of mass. The shoreline settings carry no
initialization and only two resolution levels, which assumes the inputs already
overlap -- true for b=0 volumes within one scan, but not across sessions, where
table position can differ by centimetres, more than a Rigid metric will recover
from. initialize_com is opt-in, so the DWI b=0 callers are unchanged; a test
pins that.

Also fixes a latent crash: the template feeds b0_coreg_wf, which referenced
ants_mvtc2 unconditionally and would have raised UnboundLocalError on the linear
path.
init_t2w_preproc_wf produces two T2w images on every run with a T2w, and neither
reached derivatives. t2_preproc had no consumer at all -- it was computed and
dropped. t2w_unfatsat only ever went to the DWI workflows.

They are different images and both are worth having:

  t2_preproc    the merged (unbiased) T2w template resampled into ACPC, from
                anat_reference_wf's bias_corrected output
  t2w_unfatsat  the fat-suppressed image TORTOISE T2Wreg and DRBUDDI actually
                register to. It descends from outputnode.template, so it is NOT
                bias corrected -- the SDC target and the 'preproc' T2w are not
                the same image, which is worth being able to see.

No new computation; only datasinks. The sinks need their own name node because
DerivativesDataSink takes its suffix from source_file: reusing t1_name would
emit *_T1w.nii.gz and overwrite the real T1w derivative. They are gated on
num_additional_t2ws > 0, since otherwise the sinks sit with undefined inputs and
fail at runtime.
The template -- the average of every session's b=0 reference -- was only ever
consumed by a report figure (ds_report_intramodal). The image itself was never
written, so the one product that shows how the sessions align could not be
opened.

Datasinked to the anat directory as desc-intramodal_dwiref so a single listing
shows all the subject-level products together: the T1w, both T2ws, and the b=0
average they align to. No new computation.
The intramodal template sink raised 'Could not build path with entities' and
took the whole run down: qsiprep's own path patterns (data/io_spec.json) allow
dwiref only under datatype<dwi>, so datatype='anat' had no matching pattern.

Added dwiref to the anat suffix list, which is how this spec is already
maintained -- it carries imtcoreg, another non-BIDS qsiprep suffix. That keeps
the subject-level b=0 average beside the T1w and T2ws it exists to be compared
against. Tests pin both the new path and the existing dwi/anat paths.
The sink took outputnode.intramodal_template, which is the template in its own
midpoint space -- measured ~57mm from ACPC in y on sub-0001a -- and wrote it
tagged space-ACPC. The result was byte-identical to the raw template (128x128x69
at 2.05mm, versus 193x229x193 at 1mm for the ACPC anatomicals) and did not
overlay the anatomy at all.

That is the worst kind of wrong: a file that looks like a valid derivative and
is silently in the wrong space.

init_intramodal_template_wf already computes the template->anat affine in
b0_coreg_wf; it just was not applied to anything. Added template_to_acpc, which
resamples the template through that affine onto the t1_brain grid, exposed as
outputnode.intramodal_template_acpc, and pointed the sink at it.
b0_to_intramodal_template_transforms and intramodal_template_to_t1_affine were
computed, fed into ComposeTransforms for resampling, and then discarded. The
intramodal space was a dead end: a session's native b=0 could not be mapped into
it, nor the template back to ACPC.

Writes:
  ses-N/anat/sub-X_ses-N_from-orig_to-intramodal_mode-image_xfm.mat
  anat/sub-X_from-intramodal_to-ACPC_mode-image_xfm.mat

closing the round trip BIDS b=0 -> intramodal -> ACPC -> MNI and back. No spec
change was needed: the existing xfm pattern leaves from/to unconstrained and
already places transforms under anat/, which is where the anatomical
from-orig_to-anat lives.

This also removes a blind spot. Not exporting intramodal_template_to_t1_affine
is why a template written in the wrong space -- carrying a correct-looking
space-ACPC name -- could not be checked against anything.
The test scanned a fixed 1200-character window after ds_intramodal_template,
so adding ds_intramodal_to_acpc between the node and its connect block pushed
the assertion target out of range. Check the connection tuple anywhere in the
module instead.
It flickered the session's b=0 against the group template: two different images
in two different spaces, with the 'after' frame byte-identical for every session
(average_trans.nii, the group template). Nothing in the figure varied with the
session's own registration, so it could not distinguish a good alignment from a
failed one. wm_seg was never passed, so there were no landmarks either.

Now both frames are the SAME image -- this session's b=0 -- resampled onto the
template grid, before and after its own transform. The only difference between
frames is the transform being assessed.

Landmarks come from the anatomy: b0_coreg_wf registers the template to the
anatomy, so inverting that affine carries t1_seg into template space
(MultiLabel + invert_transform_flags, the same idiom as
init_fmap_unwarp_report_wf). The ordering works out because that affine exists
once the template is built and before anything downstream needs it -- no
circular dependency, and t1_seg was already an input to the workflow.

Frames are labelled 'b=0 (header only)' and 'b=0 aligned to template'.
…ling

init_single_subject_wf raised 'Cannot make an intramodal with less than 2 groups'
whenever --intramodal-template-iters was set and a subject had one DWI group.
Cohorts routinely mix single- and multi-session subjects -- in CRASH, 24 of 59
subjects have a single session -- so one flag failed 41% of the dataset outright.

Having one group is a normal condition, not a user error. Warn, skip the
template for that subject, and process everything else normally.
Answers 'how do the individual images stack up' with numbers. On sub-0001a the
table flags ses-8 (corr 0.717 vs 0.773-0.805, corr_dev -10.1) as the only
outlier, and translation_mm independently corroborates the FOV displacement
found by hand: ses-5 and ses-8 needed the largest moves (11.7mm, 12.7mm).

Writes sub-X_desc-templateQC_dwiref.tsv and, beside it,
sub-X_space-ACPC_desc-agreement_dwiref.nii.gz (voxelwise SD/mean across the
aligned inputs). The agreement map is explicitly not a noise measure: it mixes
scan-to-scan noise, residual misregistration and genuine change.

Two robustness points, both found by testing rather than by inspection:

- ANTs writes AffineTransform_float_3_3 when registration runs float=True, so
  hardcoding the double-precision key left translation/rotation silently NaN --
  hidden further by a bare except. The key is now looked up generically and
  failures are logged: a QC column that fails quietly reads as 'nothing wrong'.

- The median absolute deviation collapses to 0 whenever inputs agree closely,
  which is the common healthy case. Scaling by it flags nothing at all, however
  extreme the outlier. Falls back to an absolute drop below the median.
diffprep_kwargs never passed num_threads, so the interface never populated
OMP_NUM_THREADS and TORTOISE helped itself to every core: a run with
--nthreads 12 --omp-nthreads 12 logged 'Using up to 24 CPU cores.'

nipype schedules against node.n_procs, so it believed DIFFPREP used 12 threads
while the process used 24. Every concurrent node contended, and any GPU/CPU
concurrency tuning would have been built on a declaration that was a factor of
two short -- lowering omp_nthreads to fit more GPU jobs would have multiplied
the oversubscription rather than balancing it.

DRBUDDI and SynthesizeDWIs already declared num_threads; DIFFPREP is the long
pole and did not. Both DIFFPREP nodes share diffprep_kwargs, so the single- and
reverse-PE paths are both covered.

Note this also matters for nipype's GPU accounting: a GPU node's demand IS its
n_procs (multiproc.py: tasks_gpu_th.append(node.n_procs)), and the per-job cost
is min(n_procs, n_gpu_procs). Concurrency is therefore governed by that ratio,
not by n_gpu_procs alone.
The previous commit claimed passing num_threads stops TORTOISE grabbing every
core. Measured on a 24-core host with a 72-volume workload, that is false:

  unconstrained                            ~2071% CPU
  OMP_NUM_THREADS=4                        ~1893%
  ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS=4   ~2003%

TORTOISEProcess spawns threads from hardware concurrency and exposes no way to
limit them -- its own --ncores help text says it applies 'ONLY to the DRBUDDI
executable and not TORTOISEProcess'.

The declaration is kept (consistent with DRBUDDI/SynthesizeDWIs, correct for
tools that honour it) but the comment now states what it does and does not do,
and records that real constraint requires cgroups.
nipype recognises a GPU node only via use_cuda or use_gpu (is_gpu_node).
SynthStrip spells its request 'gpu' and SynthSeg spells it 'cpu' (opt-out), so
neither matched: with --gpu synthseg/synthstrip they ran on the device while the
scheduler counted them as pure CPU nodes and scheduled DIFFPREP, DRBUDDI or eddy
alongside them with no coordination.

Adds a use_gpu alias to both specs, carrying no argstr -- it exists purely for
resource accounting -- set from the same gpu_enabled() call as the tool's own
flag.

DIFFPREP, DRBUDDI and ExtendedEddy already expose use_cuda and were visible.
num_threads sets OMP_NUM_THREADS, which TORTOISE overrides with
omp_set_num_threads(): measured ~1893% CPU with OMP_NUM_THREADS=4 on a 24-core
host, i.e. no effect. --ncores is read directly by the patched TORTOISE and is
an ABSOLUTE count, which matters under a batch scheduler -- the alternative,
PercentOfCpuCoresToUse, is applied to sysconf(_SC_NPROCESSORS_ONLN) and ignores
cgroup and affinity limits, so an 8-core SLURM allocation on a 128-core node
would size itself for 128.

Wired from config.nipype.omp_nthreads so nipype's n_procs declaration and the
process's actual CPU use finally agree. Requires the patched TORTOISE build.

Verified: --ncores 2 -> 202% CPU, --ncores 6 -> 701%, unset -> 2047%.
Writes sub-X_ses-Y_space-ACPC_desc-tsnr_dwi.nii.gz from the final resampled
series, so it reflects what the user receives rather than an intermediate.

Computed over b=0 volumes only. A DWI series has no resting baseline, so
'temporal' SNR is only meaningful across volumes that should look alike; the
b=0s are the only such set. Using all volumes would measure diffusion contrast
rather than noise, and would look worst exactly where the data is most
informative -- the same trap as reading FA quality from its histogram.

n_b0 is reported alongside: with few b=0 volumes the estimate is noisy. A single
b=0 yields an explicit zero map and a warning rather than a silently
meaningless one.
A string match passes just as happily when the inputnode field it names has
been renamed. Assert the actual edges and sink entities instead, so a broken
connection fails in the suite rather than eight hours into a run.
Two call sites, one root cause. N4 fits its bias field by least squares on log
intensities, so voxels near zero become enormous negative outliers, and least
squares is not robust to them. The fit goes near-degenerate rather than merely
noisy -- which is why it diverges on one session and not the next from inputs
that are statistically indistinguishable.

Caught on CRASH sub-2463p ses-1: the field spanned 98x inside the brain
(superior/inferior 2.69) against 1.5x for the same subject's ses-2. The superior
cortex was crushed ~4x in the final series, the brain mask then followed the
signal that was destroyed and dropped 128k voxels, and every downstream
derivative inherited it. The raw acquisition was clean (inferior/superior 1.08x
vs 0.96x) and so were the inputs to the node -- mask 200334 vs 199588 voxels,
Dice 0.941 vs 0.947 against an intensity-only brain estimate.

DWI (interfaces/bias.py, wired in init_finalize_denoising_wf):
dwibiascorrect hands the brain mask to N4 as a WEIGHT image (-w, never -x; see
mrtrix3/dwibiascorrect/ants.py), so its binary values are the per-voxel weights
of that least-squares fit. N4WeightMask zeroes in-mask voxels below 2x
background before the mask is passed as -mask. On the failing session that drops
1.68% of the mask and takes the field from 98x/2.69 to 1.9x/0.84; the healthy
session moves 1.5x/0.91 -> 1.6x/0.88, i.e. not perturbed. It refuses to discard
more than half the mask, since a mostly-dim mask means something upstream is
already wrong and shrinking it would hide that.

Anatomical (init_anat_template_wf, both the single- and multi-image paths):
prepend ImageMath TruncateImageIntensity 0.01 0.999 256, which is what
niworkflows does ahead of every N4 call it makes (niworkflows/anat/ants.py:237).
qsiprep was calling N4 bare and had the same exposure.

Ruled out along the way, each by measurement rather than argument: a bad input
mask; bad input data; B-spline extrapolation into air (adding -x changes the
in-mask field by exactly nothing -- with -w supplied only non-zero weights drive
the fit either way); the two-level -c (one level was slightly worse); and -b 150
vs 100 (identical lattice at this FOV).
The anatomical merge and the intramodal b=0 template were using
shoreline_*_{Rigid,Affine}.json, which is tuned for within-scan b=0 motion
correction: noisy, 2 mm, contrast that varies between volumes. Template creation
runs on high-SNR images that share contrast and inherited those settings by
accident.

One parameter accounts for almost all of the cost. A convergence threshold of
1e-08 with a window of 20 never fires, so every registration burns its full
1000x1000 iteration budget long after it has converged.

Measured on the real inputs (0.94 mm T1w against the merge template, three
images, ITK threads fixed at 4):

  arm                      time    speedup   rot vs base   trans    final MI
  1e-08 (as shipped)     ~119 s      1.0x             --      --    -0.98862
  1e-06                   ~7.3 s     16.4x       <0.028d  <0.008mm  -0.98862

The metric value is identical to five decimal places on every image, with the
same metric configuration -- so this is the same optimum, reached sooner, not a
nearby one. Other candidates were measured and rejected: 48->32 bins bought 1.2x,
Random->Regular 1.8x, and a 32-bin/Regular/3-level combination was 1.3x faster
than 1e-06 alone but moved the transform up to 0.089d with an MI that is not
directly comparable. Not worth it on top of 16x.

Scope: new unbiased_template_* files rather than editing shoreline_*, because
1e-08 may well be defensible for SHORELine's actual job. init_b0_hmc_wf gains a
"settings" argument defaulting to "shoreline", so within-scan HMC is unchanged;
a test pins that. The new files also drop per-stage list entries that describe
stages they do not have -- nipype silently truncated them, so they never ran.

Reaches: the anatomical merge (T1w and T2w), and the intramodal template when
--intramodal-template-transform is Rigid or Affine. NOT BSplineSyN, which is
built by antsMultivariateTemplateConstruction2 and never goes through
init_b0_hmc_wf; a test documents that boundary.

init_qsiprep_intramodal_template_wf is left unwired. It is dead for every
transform setting -- its def is the only occurrence in the tree -- and it passes
spatial_bias_correct= to init_b0_hmc_wf, which does not accept it, so it would
raise TypeError if it ever ran. Adding the new settings there would imply it
works.
The truncation added in d9504a3 fed N4 and then propagated N4's output image
downstream, so the clipping ended up in the merge template and in
desc-preproc_T1w/T2w. Measured on a real 0.94mm T1w, that is not cosmetic: the
0.999 clip drops the ceiling from 1404 to 457 and flattens 12845 brain voxels
(the top 0.4%, above the 99.6th percentile of brain tissue) to a constant.

Estimate on the truncated image, apply the field to the original:

  original                max 1404.0   in-brain p99.9  629.0
  before (N4 on trunc)    max  411.8   in-brain p99.9  344.4   <- capped
  after  (original/bias)  max 1003.6   in-brain p99.9  467.1

|after - original/bias| is 0 in brain, the field is 1.03-2.03 there, and the
correction still works (in-brain CoV 0.4247 -> 0.3759).

This is what niworkflows actually does -- its truncated image feeds only the
preliminary N4 that drives brain masking, while the bias_corrected output comes
from N4 run on the untruncated original (niworkflows/anat/ants.py: inu_n4 vs
inu_n4_final). I had cited niworkflows as justification while doing something it
does not do.

It also makes the two paths consistent: dwibiascorrect already divides the full
original DWI series by the estimated field, so nothing truncated or masked
reaches the DWI output either. There the mask only conditions N4's -w weights
(and, second order, the global rescale integral).
DIFFPREP does not offload the series to the GPU the way eddy_cuda does. It runs
the GPU as one more worker alongside the CPU threads, splitting each pass:

    npass = ceil(nvolumes / (ngpus * ratio + ncores - ngpus))

Per pass the GPU takes `ratio` volumes and each CPU thread takes one. With 279
volumes, one GPU and --omp-nthreads 24 that leaves 39% of the series on the GPU
and 61% on the cores -- which is why TORTOISEProcess wants many CPUs while
eddy_cuda is content with one.

Upstream hardcodes the ratio to 15. It describes the machine, not the data:
roughly how many volumes the GPU gets through while one core does one. A node
pairing a datacentre card with a small CPU allocation wants a much larger value
and otherwise leaves the GPU idle; a weak or shared GPU wants a smaller one and
otherwise makes the GPU the straggler that every pass waits on.

Forwarded only when explicitly set, so TORTOISE's own default stands rather than
qsiprep inventing one. Requires the patched TORTOISE (--gpu_cpu_ratio, commit
2fe80a5 in the fork).

Its influence falls as --omp-nthreads rises -- about 68% of volumes at 8 cores,
39% at 24, 19% at 64 -- so it matters most on small allocations and single-GPU
desktops, and barely at all on a large cluster node. Documented that way in the
help text, since the instinct is to reach for it in exactly the opposite case.
TORTOISEConvert decompresses a whole 4D series into an uncompressed .nii --
about 1.3 GB of output for a 279-volume acquisition. It declared no mem_gb, so
nipype budgeted the 0.20 GB default and scheduled it purely against cores: with
24 cores and three subjects' worth of ready nodes, eight ran at once.

The kernel OOM-killed one at 1.69 GB RSS eleven hours into a 3-subject run
(global_oom, task_memcg=docker). nipype's process pool never recovers from that
-- every subsequent callback raised BrokenProcessPool and the workflow sat idle
until it was stopped, having completed 0 of 24 DIFFPREP nodes.

Two subjects fit and three did not, which is why this survived the earlier runs.

Declared 2.0 GB, above the 1.69 GB the kernel actually observed. Both
construction sites are covered: the plain path and the per-group rpe_series
path. A test asserts every TORTOISEConvert node carries a mem_gb, since the
failure mode is silent until the box happens to be full.

Note this only binds if the caller also gives nipype a memory budget --
--mem-mb, otherwise it assumes all of system RAM with no headroom.
The 2.0 GB in the previous commit was right for CRASH by luck, not by
construction. TORTOISEConvert does load_img(dtype='float32') and writes float32,
so its working set is nvoxels*4 whatever the input dtype -- a longer or
higher-resolution acquisition would have blown the same budget.

Neither obvious shortcut is usable. CRASH is uint16 on disk, so sizing from the
array dtype understates by 2x; _create_mem_gb's 'filesize' is os.path.getsize on
the *gzipped* file and understates by 3.4x. Ground truth on one session:
0.34 GB gzipped, 0.59 GB as uint16, 1.17 GB as float32 -- the last matching the
converted .nii exactly.

tortoise_convert_mem_gb() computes nvoxels*4*1.5, the 1.5 covering the transient
source array and interpreter overhead that put the OOM-killed process at 1.69 GB
RSS against a 1.17 GB array. For CRASH that yields 1.76 GB.

scan_groups reaches init_diffprep_hmc_wf, so the sizes are available at workflow
build time; _build_rpe_diffprep_stage takes it as a parameter like n_procs.
Unreadable paths fall back rather than raising, since docs builds pass paths that
do not exist.

Tests pin the dtype-independence, the linear scaling, the sum over inputs, and
that the CRASH geometry still covers the 1.69 GB the kernel observed -- and that
the hardcoded value does not come back.
tsnr.nii.gz (386 B), n4_weights.nii.gz (157 B) and template_agreement.nii.gz
(59 KB) are outputs from running SimpleInterfaces by hand: nipype writes to
runtime.cwd, which was the repo root, and a subsequent 'git add -A' swept them
in alongside the real changes.

Removed from the tree and ignored so it cannot recur. Deliberately NOT rewriting
history: the three files total ~60 KB, they are already on origin, and rewriting
21 published commits to reclaim that much would cost every clone a hard reset for
no practical gain. If they had been large enough for GitHub to care, or had
contained anything sensitive, the trade would go the other way.
test_tsnr, test_n4_robustness and test_template_qc each called
<Interface>(...).run() with no cwd, so nipype wrote tsnr.nii.gz,
n4_weights.nii.gz and template_qc.tsv into the process working directory.
CI mounts the checkout read-only and cds into it
(-v /tmp/src/qsiprep:/tmp/src/qsiprep:ro -w /tmp/src/qsiprep), so all
eleven of those tests died with "OSError: [Errno 30] Read-only file
system". unit_tests has been red on this branch since the tests landed --
it failed on d45bae5 and again on 62cb9ea.

The same root cause is why 477ae4b had to delete n4_weights.nii.gz,
template_agreement.nii.gz and tsnr.nii.gz from the repo root and gitignore
them: locally the writes succeed and land in the source tree. That commit
treated the symptom; the tests kept writing there.

Pass cwd=tmp_path, which is what test_interfaces_freesurfer,
test_interfaces_mrtrix3, test_interfaces_dipy and test_interfaces_diffprep
already do, and which keeps the outputs out of the working tree entirely.
The branch predates the ruff pin alignment, so `ruff check .` reported 14
errors and `ruff format --diff .` wanted 12 files reformatted under the
0.15.21 that lint.yml, .pre-commit-config.yaml and pyproject.toml now all
name. That would fail Contribution checks as soon as this PR retargets.

Auto-fixable: import sorting in template_qc.py, diffprep.py and
test_interfaces_diffprep.py, plus the formatter's own line joins and
kwarg splits -- all cosmetic, no behaviour touched.

By hand:
  * three dict() calls in test_t2w_derivatives.py become literals (C408);
  * a compound assert in test_tsnr.py splits in two so a failure says
    which node is missing (PT018);
  * one connect() line in volume.py wraps, since its block carries
    `# fmt:skip` and the formatter leaves it alone (E501).
…biascorr-masks

# Conflicts:
#	qsiprep/workflows/anatomical/volume.py
Three related breaks, all confirmed in the intramodal_template CI job:
TemplateQC's mandatory aligned_images was never connected on the
BSplineSyN/SyN branch (mvtc2 exposes no per-input aligned images), the
per-group orig->intramodal sink received a nonlinear [affine, warp] pair
that cannot fit a single-file .mat pattern, and the finalize reportlet
nodes were built even when the template was skipped for single-group
subjects, leaving their inputs undefined. TemplateQC and the transform
export are now linear-only, and finalize takes make_intramodal_template
so the skip actually skips.
The first-lex path silently reverted the anat merge to the shoreline
registration settings while the iterative path used unbiased_template.
SplitDWIsByDistortionGroup, ConcatenateDIFFPREPGroups, DIFFPREPSplitOutputs
and SynthesizeDWIs all hold at least one whole series as float32 but ran at
nipype's 0.2 GB default; give them the same data-derived budget as
TORTOISEConvert. Also read the reverse-PE series list from fieldmap_info,
where it actually lives -- the top-level key never existed, so the rpe
volumes were silently left out of the budget.
desc-tsnr with a dwi suffix claimed a 3D map was a DWI series; use the
same stat-/dwimap pattern as the CNR map. The b=0 count and median now
travel in the sidecar. Every integration test checks its output list, so
add the new map (and the intramodal template products) to all of them --
the missing entries are what failed the current CI run.
Move --tortoise-gpu-cpu-ratio into the motion-correction group next to
--diffprep-config, reword the --anat-biascorrect help (console NORM does
not remove the need for N4; auto/none are for deliberately skipping it),
and restore b1_biascorrect_stage's docstring pairing in config.py.
Remove measurement war stories, session identifiers, and change-history
narration from comments and docstrings; keep only what explains genuinely
counterintuitive behavior. Also restore io_spec.json's original formatting
(the real change is one line), dedupe the _get_first helper, narrow two
broad excepts, run the new interface test in tmp_path, and replace the
source-introspection meta-test with a behavioral assertion.
@codecov-commenter

codecov-commenter commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.94517% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.62%. Comparing base (785eba5) to head (4c8a150).

Files with missing lines Patch % Lines
qsiprep/workflows/anatomical/volume.py 75.30% 19 Missing and 1 partial ⚠️
qsiprep/interfaces/template_qc.py 85.71% 6 Missing and 6 partials ⚠️
qsiprep/workflows/base.py 52.94% 6 Missing and 2 partials ⚠️
qsiprep/interfaces/bias.py 92.85% 2 Missing and 2 partials ⚠️
qsiprep/workflows/dwi/hmc.py 86.95% 3 Missing ⚠️
qsiprep/interfaces/tsnr.py 96.22% 1 Missing and 1 partial ⚠️
qsiprep/workflows/dwi/derivatives.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           tortoiseproc    #1078      +/-   ##
================================================
+ Coverage         50.19%   51.62%   +1.43%     
================================================
  Files                68       71       +3     
  Lines             10565    10870     +305     
  Branches           1180     1212      +32     
================================================
+ Hits               5303     5612     +309     
+ Misses             5000     4995       -5     
- Partials            262      263       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

After the predicted-shell workflow was removed, the shelled/non-shelled
classification no longer selected a path: every reverse-PE series takes
the stock DRBUDDI route, and DRBUDDI shell synthesis runs only when the
user sets drbuddi_synth_shell_bval. The detector's sole remaining effect
was an advisory log line, and the config key only overrode that
classification -- redundant with the explicit opt-in.

Removes _rpe_series_is_shelled, _side_is_shelled, _sibling_bval, the
config default, the log message, and the detector's tests. The
stock-path workflow test no longer needs a config file to exercise the
non-shelled case, since the path never depended on the classification.
@mattcieslak
mattcieslak merged commit 713b370 into main Aug 11, 2026
27 checks passed
@mattcieslak
mattcieslak deleted the anat-biascorr-masks branch August 11, 2026 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants