Return status bug (incorrectly reported error) v1.0.20240202

[tomhampshire]

Describe the bug

Using:

Chris Rorden's dcm2niiX version v1.0.20240202 GCC13.2.1 x86-64 (64-bit Linux)

Running dcm2niiX on a DICOM dataset produces an error status of (incorrectly) 1, whereas in:
Chris Rorden's dcm2niiX version v1.0.20230411 (JP2:OpenJPEG) (JP-LS:CharLS) GCC8.4.0 x86-64 (64-bit Linux)
the error status is (correctly) 0.

Only warnings are flagged, but the data are processed successfully in each case.

To reproduce

Steps to reproduce the behavior:

1. Install v1.0.20240202 GCC13.2.1 x86-64 (64-bit Linux) (installed from pip using `dcm2niix @ git+https://github.com/rordenlab/dcm2niix@v1.0.20240202')
2. Run the command dcm2niix -p n -z y -b y -ba y -f %s_%p_%t_%d -o OUTPUT INPUT
3. Check the return status code (OS/shell specific)

We are using patient data to produce this error, so we don't have the permission to share it. We can help debug this problem remotely, if helpful. Here is the output of the generated .json sidecar:

{
	"Modality": "MR",
	"MagneticFieldStrength": 3,
	"ImagingFrequency": 127.74872,
	"Manufacturer": "Philips",
	"ManufacturersModelName": "DicomCleaner",
	"DeviceSerialNumber": "SN000000",
	"StationName": "EDUROAM-INT-DHCP",
	"BodyPartExamined": "PROSTATE",
	"PatientPosition": "FFS",
	"SoftwareVersions": "Thu Jun 25 08:55:38 EDT 2020",
	"MRAcquisitionType": "2D",
	"SeriesDescription": "DWI 0 150 500 1000 wREST",
	"ProtocolName": "DWI 0 150 500 1000 wREST",
	"ScanningSequence": "SE",
	"SequenceVariant": "SK",
	"ScanOptions": "FS",
	"PulseSequenceName": "DwiSE",
	"ImageType": ["ORIGINAL", "PRIMARY", "DIFFUSION", "NONE"],
	"SeriesNumber": 1201,
	"AcquisitionTime": "13:52:10.690000",
	"AcquisitionNumber": 12,
	"PhilipsRescaleSlope": 0.89939,
	"PhilipsRescaleIntercept": 0,
	"PhilipsScaleSlope": 0.118242,
	"UsePhilipsFloatNotDisplayScaling": 0,
	"SliceThickness": 5,
	"SpacingBetweenSlices": 5,
	"NumberOfAverages": 4,
	"EchoTime": 0.078,
	"RepetitionTime": 4,
	"MTState": false,
	"FlipAngle": 90,
	"CoilString": "MULTI COIL",
	"PercentPhaseFOV": 137.5,
	"PercentSampling": 101.136,
	"EchoTrainLength": 105,
	"PartialFourierDirection": "PHASE",
	"PartialFourierEnabled": "YES",
	"PhaseEncodingStepsNoPartialFourier": 178,
	"FrequencyEncodingSteps": 176,
	"PhaseEncodingStepsOutOfPlane": 1,
	"AcquisitionMatrixPE": 178,
	"ReconMatrixPE": 352,
	"ParallelReductionFactorInPlane": 1.7,
	"ParallelAcquisitionTechnique": "SENSE",
	"WaterFatShift": 37.7856,
	"EstimatedEffectiveEchoSpacing": 0.000245509,
	"EstimatedTotalReadoutTime": 0.0861735,
	"AcquisitionDuration": 308,
	"PixelBandwidth": 1423.09,
	"PhaseEncodingAxis": "j",
	"ImageOrientationPatientDICOM": [
		0.999906,
		-0.0011492,
		-0.0136693,
		0.00118002,
		0.999997,
		0.00224705	],
	"InPlanePhaseEncodingDirectionDICOM": "COL",
	"BidsGuess": ["dwi","_acq-SKSEDwiSEp17_run-1201_dwi"],
	"ConversionSoftware": "dcm2niix",
	"ConversionSoftwareVersion": "v1.0.20240202"
}

and the bval file:

0 2000 2000 2000

and the bvec file:

0 0 0 0
0 0 0 0
0 0 0 0

Expected behavior

An error code of 0 should be produced

Output log

Chris Rorden's dcm2niiX version v1.0.20240202  GCC13.2.1 x86-64 (64-bit Linux)
Found 1 DICOM file(s)
::autoBids:Philips pulseSeq:'DwiSE' scanSeq:'SE' seqVariant:'SK'
Warning: Isotropic DWI series, all bvecs are zero (issue 405)
Check bvecs: expected Patient Position (0018,5100) to be 'HFS' not 'FFS'
Warning: Volume 1 appears to be derived image ADC/Isotropic (non-zero b-value with zero vector length)
Warning: Volume 2 appears to be derived image ADC/Isotropic (non-zero b-value with zero vector length)
Warning: Volume 3 appears to be derived image ADC/Isotropic (non-zero b-value with zero vector length)
Warning: Saving 4 DTI gradients. Validate vectors (image slice orientation not reported, e.g. 2001,100B).
Philips Scaling Values RS:RI:SS = 0.89939:0:0.118242 (see PMC3998685)
Convert 1 DICOM as /tmp/tmpe_1d9a87/1201_DWI_0_150_500_1000_wREST_20230419130502_DWI_0_150_500_1000_wRESTe (352x352x17x4)
Conversion required 1.042489 seconds (1.022187 for core code).
CompletedProcess(args=['/home/tom/code/GSP/dicom-tools/.venv/lib/python3.9/site-packages/dcm2niix/dcm2niix', '-p', 'n', '-z', 'y', '-b', 'y', '-ba', 'y', '-f', '%s_%p_%t_%d', '-o', '/tmp/tmpe_1d9a87', '/tmp/tmpz4k41ykq'], returncode=0)

NOTE: the above states a return code of 0, however, a return code of 1 is produced

Version

Chris Rorden's dcm2niiX version v1.0.20240202 GCC13.2.1 x86-64 (64-bit Linux)

Troubleshooting
The issue is not resolved using the latest commit of dcm2niix: e2ead4b

[tomhampshire]

Is it possible that the bvector extraction is triggering the failed status code?

[tomhampshire]

As a suggestion, could you check your return status codes as part of your QA script to ensure all (valid) datasets return 0?
https://github.com/neurolabusc/dcm_qa/blob/master/batch.sh

[neurolabusc]

@tomhampshire the dcm_qa script batch.sh is designed to ensure identical output to a previous reference run. However, the BIDS standard is evolving, so newer versions add new features yielding non-identical results. This is why the development branches can generate non-zero exit codes for the validation script, this is the expected behavior that requires a human to ensure the changes are valid.

Converting this datasets directly with dcm2niix does return a valid exit code.

$ dcm2niix ~/src/dcm_qa
Chris Rorden's dcm2niiX version v1.0.20240202  (JP2:OpenJPEG) (JP-LS:CharLS) Clang12.0.0 ARM (64-bit MacOS)
...
Convert 2 DICOM as /Users/chris/src/dcm_qa/dcm_qa_ax_asc_35sl_20140310133834_6a (64x64x35x2)
CSA slice timing based on 2nd volume, 1st volume corrupted (CMRR bug, range 0..4880, TR=3000 ms)
Convert 2 DICOM as /Users/chris/src/dcm_qa/dcm_qa_fMRI_MB_asc_20140310133834_25a (86x86x36x2)
Conversion required 1.531967 seconds (1.422817 for core code).
$ echo $?
0

Without having access to exemplar data, it is hard to help diagnose your problem. I would certainly be careful with diffusion data acquired using foot first supine - BIDS/FSL bvec files are in image space, while Philips generates world space coordinates. See the validate bvec details. I would work with the Philips Clinical Scientist associated with your site to resolve this. You can always bisect the commits to the dcm2niix code base to determine the precise change that elicits the change in behavior. However, given the unusual nature of your images, a non-zero exit code may well be warranted as I have never validated this style of diffusion acquisition from this manufacturer.

[tomhampshire]

Hi @neurolabusc - thanks for your response.
I can send you directly a dataset that replicates this issue, but ask that it isn't distributed/shared/exposed publicly. Could you please provide an email address/private data upload link that I can use?

[tomhampshire]

Hi - an update. We've found some DICOM data that can be linked to this issue and replicates the issue.
You'll see that the stdout reports that there is a return status code of 0, however, on inspecting this value, it is 1. No error has actually occurs and the relevant files have been successfully generated.
https://seafile.goldstandardphantoms.com/f/ad439be4b2524ac8af89/

[neurolabusc]

Unable to replicate. Can you provide the precise command you are supplying? Does the problem persist when you compile dcm2niix locally (to ensure it matches your library versions)?

 $ dcm2niix_stable -p n -z y -b y -ba y -f %s_%p_%t_%d -o ~/neuro/issue811  ~/neuro/issue811
Chris Rorden's dcm2niiX version v1.0.20240202  (JP2:OpenJPEG) (JP-LS:CharLS) Clang12.0.0 ARM (64-bit MacOS)
Found 1 DICOM file(s)
::autoBids:Philips pulseSeq:'DwiSE' scanSeq:'SE' seqVariant:'SK'
Note: B0 not the first volume in the series (FSL eddy reference volume is 15)
Note: 5 volumes appear to be ADC or trace images that will be removed to allow processing
Warning: Saving 16 DTI gradients. Validate vectors (image slice orientation not reported, e.g. 2001,100B).
Philips Scaling Values RS:RI:SS = 1.48205:0:0.0263705 (see PMC3998685)
Convert 1 DICOM as /Users/chris/neuro/issue811/1101_DWI_20210526171028_DWIb (64x64x40x21)
Conversion required 0.312692 seconds (0.312649 for core code).
$ echo $?                                                                                  
0

[tomhampshire]

Hi. I believe the issue is actually with the python package. When installed with, for example, pip install git+https://github.com/rordenlab/dcm2niix@master, dcm2niix is then available on the PATH as a Python script. The Python script does not correct return the status code of dcm2niix.
I have a patch which, I believe, should fix it (see 8179dec). However, I am having an issue testing it. If I pip install this local edit, the installed dcm2niix Python script does not contain the referenced code. The script always contains:

#!/path/to/.venv/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from dcm2niix import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

I cannot find where this code is coming from, and am unable to make the patch work. If someone is able to point me in the right direction, I will supply a pull request.

[captainnova]

I cannot find where this code is coming from, and am unable to make the patch work. If someone is able to point me in the right direction, I will supply a pull request.

I am unfamiliar with dcm2niix's included python wrapper, but AFAICT it is in dcm2niix/__init__.py

[tomhampshire]

@captainnova - that's correct. This gets called from dcm2niix/__main__.py which is the script entry point. This is the file that I've editing in my patch (see 8179dec#diff-af5a857cac7b4f647a0977d18e1331ab44b21e083c3ae9453455c58b0ddc80c2), but when building and installing the dcm2niix Python package (from my locally changed codebase), the following is always installed as the dcm2niix entrypoint:

#!/path/to/.venv/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from dcm2niix import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

I would suggest that the final line is changed to sys.exit(main().returncode), as main() returns a subprocess.CompletedProcess object, not a return code; however, I am unable to make a change to this installed entrypoint script.
This may be due to an unfamiliarity with your build system, as, when searching your codebase, I cannot find the above script anywhere (grep'ing for sys.exit, for example, does not reveal its location)

[tomhampshire]

On the assumption the above script is auto-generated by part of the build system, I'd suggest that the main function returns the integer return status, rather than a subprocess.CompletedProcess object

[tomhampshire]

This pull request allows the Python console script to return the correct exit code from the dcm2niix executable: #815

[captainnova]

On the assumption the above script is auto-generated by part of the build system, I'd suggest that the main function returns the integer return status, rather than a subprocess.CompletedProcess object

That seems reasonable to me, but I do not understand exactly what the python build/packaging is doing either. Maybe @casperdcl would care to comment?