Skip to content

ci/compare_maps.py: zero scl_slope should disable the intensity transform, not multiply voxels by zero #37

Description

@mathieuboudreau

Summary

ci/compare_maps.py:read_nii applies the NIfTI intensity transform when scl_slope == 0.0 and scl_inter != 0.0, multiplying every voxel by zero. Per NIfTI-1, a zero scl_slope means the transform is unset and must be skipped entirely — intercept included.

The comment directly above the check already states the correct rule; only the condition disagrees with it.

https://github.com/qMRLab/qmrust/blob/main/ci/compare_maps.py#L37-L41

# NIfTI intensity transform: displayed = slope * stored + intercept. A zero
# (unset) slope means no scaling, per the spec — treat it as identity.
if scl_slope not in (0.0, 1.0) or scl_inter != 0.0:
    return [scl_slope * v + scl_inter for v in vals]
return list(vals)

Why it fires

With scl_slope = 0.0, scl_inter = 5.0:

clause result
0.0 not in (0.0, 1.0) False
5.0 != 0.0 True
False or True True → transform runs

giving 0 * v + 5 for every voxel.

Reproduction

Against ci/compare_maps.py at main, on a .nii.gz whose header carries scl_slope=0.0, scl_inter=5.0 over data [3.0, 4.0]:

read_nii(...) -> [5.0, 5.0]
NIfTI-1 requires -> [3.0, 4.0]

Impact

Currently latent, not active. The pinned OSF archives don't set a zero slope with a nonzero intercept, so today's comparisons are unaffected and no published result is wrong.

It matters because of how it would fail rather than how likely it is. A map read this way collapses to a constant: the correlation against the reference goes to zero and compare_maps.py exits non-zero — but the failure reads as "qmrust disagrees with qMRLab", pointing at the fit rather than at the reader. The risk grows as the script is pointed at maps from tooling that wasn't written alongside it (DICOM→NIfTI conversions do emit intercepts, and some writers leave scl_slope at 0 to mean "unset").

Suggested fix

Test the sentinel before the identity shortcut:

# A zero slope means "unset" and disables the transform ENTIRELY -- including any
# intercept. Folding this into the condition below would multiply by zero whenever
# scl_inter is nonzero, which is the one case the spec explicitly forbids.
if scl_slope == 0.0:
    return list(vals)
if scl_slope != 1.0 or scl_inter != 0.0:
    return [scl_slope * v + scl_inter for v in vals]
return list(vals)

Context

Found while building qMRLab/qmrlab_ci, whose NIfTI reader was derived from this one and inherited the same condition. Fixed there in qMRLab/qmrlab_ci@57ba9c3 with a regression test covering slope=0, inter≠0.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions