MR::Axes::permutations_type: Fix is_identity() #3169
Conversation
- Increment version number in CMakeLists.txt to reflect #3101 doing a merge with tag 3.0.5. - Various code features overlooked during writing of 1ebdb39. - Change MR::Axes::permutations_type to be a class rather than typedef in order to define a default uninitialised value, and also make it uint8 rather than size_t given it only has to stores the values 0, 1, 2, and an invalid flag. - Fix check for shuffle being the identity matrix as per #3169.
|
Should look into whether the effect here can be demonstrated. |
|
As best I can tell, the bug should be inconsequential from a data perspective. Python snippet used to make sure that NIfTI writes were not detrimentally affected:#!/usr/bin/env python3
import os
import subprocess
import sys
PATHS = {
"3.0.4": "/home/unimelb.edu.au/robertes/src/3.0.4/bin",
"3.0.5": "/home/unimelb.edu.au/robertes/src/3.0.5/bin",
"bugfix": "/home/unimelb.edu.au/robertes/src/master/bin"
}
STRIDES = [
"+1,+2,+3",
"+1,+2,-3",
"+1,-2,+3",
"+1,-2,-3",
"-1,+2,+3",
"-1,+2,-3",
"-1,-2,+3",
"-1,-2,-3",
"+1,+3,+2",
"+1,+3,-2",
"+1,-3,+2",
"+1,-3,-2",
"-1,+3,+2",
"-1,+3,-2",
"-1,-3,+2",
"-1,-3,-2",
"+2,+1,+3",
"+2,+1,-3",
"+2,-1,+3",
"+2,-1,-3",
"-2,+1,+3",
"-2,+1,-3",
"-2,-1,+3",
"-2,-1,-3",
"+2,+3,+1",
"+2,+3,-1",
"+2,-3,+1",
"+2,-3,-1",
"-2,+3,+1",
"-2,+3,-1",
"-2,-3,+1",
"-2,-3,-1",
"+3,+2,+1",
"+3,+2,-1",
"+3,-2,+1",
"+3,-2,-1",
"-3,+2,+1",
"-3,+2,-1",
"-3,-2,+1",
"-3,-2,-1",
"+3,+1,+2",
"+3,+1,-2",
"+3,-1,+2",
"+3,-1,-2",
"-3,+1,+2",
"-3,+1,-2",
"-3,-1,+2",
"-3,-1,-2",
]
for version, binpath in PATHS.items():
errors = []
os.makedirs(version)
for intended_strides in STRIDES:
image_path = os.path.join(version, intended_strides.replace('+', 'p').replace('-', 'm') + '.nii')
subprocess.run([os.path.join(binpath, 'mrconvert'),
'in.mif',
image_path,
'-strides', intended_strides,
'-quiet'])
# Make sure that *upon reorientation at image load*
# the strides reported by mrinfo match what they should be
actual_strides = subprocess.run([os.path.join(binpath, 'mrinfo'),
image_path,
'-strides',
'-config', 'RealignTransform', 'true'],
capture_output=True).stdout.decode().strip()
if [int(item) for item in actual_strides.split(' ')] != [int(item) for item in intended_strides.split(',')]:
errors.append((intended_strides, actual_strides))
if errors:
sys.stderr.write(f'{len(errors)} errors for version {version}:\n')
for error in errors:
sys.stderr.write(f' {error[0]} != {error[1]}\n')
else:
sys.stderr.write(f'No errors for version {version}\n') |
|
Error came specifically from 79c4ba4. |
Oversight in #3011. Believe that at some point I changed the function from checking for a non-identity transform to checking for an identity transform, and one of the operators didn't get changed.
This should affect any NIfTI export. My best guess right now regarding how this could have survived my barrage of tests is that the metadata checks had no issue in terms of concordance with the image data, but the image data themselves were not conforming to the axis strides that were being requested, and the metadata tests in github.com/Lestropie/DWI_metadata were not doing any check for that and the CI tests in MRtrix3 don't have adequate coverage.