remove bval bvecs files generated for SBRef on XA60/CMRR/dcm2niix

[bpinsard]

Weirdly XA60 data that I got recently started to produce bvals/bvecs files for SBRef from dcm2niix.
I don't know where the changes originates, so better filter more aggressively here, as the only files allowed to have such sidefiles are _dwi suffixed ones.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.55%. Comparing base (9e1e601) to head (f349ed2).
Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #819      +/-   ##
==========================================
+ Coverage   82.48%   82.55%   +0.06%     
==========================================
  Files          42       42              
  Lines        4323     4340      +17     
==========================================
+ Hits         3566     3583      +17     
  Misses        757      757              

☔ View full report in Codecov by Sentry.
📢 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.

[yarikoptic]

for what types of files it started to being produced? might be better to get to the bottom of it may be?

[bpinsard]

ah yes sorry 'SBRef' from CMRR sequence

[yarikoptic]

@bpinsard was there dcm2niix version change?
@neurolabusc do you know anything about recent changes to CMRR SBRef?

[neurolabusc]

@yarikoptic if you want feedback create an issue on the dcm2niix github page and send an example image that demonstrates the behavior to my institutional email. One challenge is that some DWI b=0 sequences are identical to a spin-echo phase-encoding polarity (PEpolar) scan use to remove gradient distortions from a gradient echo EPI scan. Therefore, for some sequences one often needs to know the intention, which is where dcm2niix wrappers like heudiconv play a role.

[bpinsard]

It is just the SBRef from CMRR causing that issue. "BidsGuess": ["dwi","_acq-epse2p2_dir-AP_run-35_sbref"], show that it is clearly identified as sbref.
I can go acquire phantom data if necessary.
I agree with @neurolabusc that this is mainly a BIDS/heudiconv issue, and if we fix it here we will have fewer issues in the future.

I am not sure which tag is used in dcm2niix for XA data to decide when to not export bvecs/bvals, but the mrprotocol shows the 3 bvalues configured for the sequence, and is exactly the same as for the multiband series.

gdcmdump --mrprot  ./MRe.1.3.12.2.1107.5.2.43.167006.2025031809361117813976411 | grep Diffusion
sDiffusion.alAverages.__attribute__.size : 128
sDiffusion.alAverages[0] : 1
sDiffusion.alAverages[1] : 1
sDiffusion.alAverages[2] : 1
sDiffusion.alAverages[3] : 1
sDiffusion.alBValue.__attribute__.size : 128
sDiffusion.alBValue[1] : 300
sDiffusion.alBValue[2] : 1000
sDiffusion.alBValue[3] : 2000

The ImageType show DIFFUSION for the multiband series and not for the SBRef.

[yarikoptic]

ok, let's get back to

I don't know where the changes originates, so better filter more aggressively here, as the only files allowed to have such sidefiles are _dwi suffixed ones.

According to https://bids-specification.readthedocs.io/en/stable/modality-specific-files/magnetic-resonance-imaging-data.html#case-4-multiple-phase-encoded-directions-pepolar and @neurolabusc description above on when produced -- they are also allowed for fmap/..._epi. , see schema also , so shouldn't we actually fix up for this instead (as to add handling of fmap/ folder or _epi suffixes) or am I missing the point?

[bpinsard]

The bval / bvecs should only be kept for _dwi suffixes, that can only happen in sub_ses/dwi folder, so if we remove all other bvecs/bvals we should cover any future case of a non-dwi sequence wronlgy producing bvals/bvecs (or someone using a dwi sequence to do non-dwi measures,).

The fmap case was already covered and is still covered with that patch, as it will continue to remove bvecs/bvals for _epi suffix.

[yarikoptic]

Sorry for being anal -- just trying to get a better grasp.

With this change I think the else: warning on

                lgr.warning(
                    DW_IMAGE_IN_FMAP_FOLDER_WARNING.format(folder=prefix_dirname)
                )

might be confusing false positive since we might be finding _dwi somewhere else than dwi/ folder (e.g. due to a bug in heuristic or alike)

Also, how/where do we treat fmap/..._epi here -- from the code it seems we could also run into the warnings, can't we?

[bpinsard]

I see you're right that the logic has drifted, so we can change that message to

DW_IMAGE_WRONG_SUFFIX_WARNING = (
    "Diffusion-weighted image saved as non dwi ({prefix})"
)

and

                lgr.warning(
                    DW_IMAGE_WRONG_SUFFIX_WARNING.format(prefix=prefix)
                )