Enhanced dicom issues

[akhanf]

With our recent Siemens software upgrade and move to enhanced dicoms we expected some issues, and here are the ones I've seen so far.

1. the cfmm2tar filename pattern, specifically the study instance uid hash, is of a different form now:

e.g. from:
Lau_NeuroAnalytics_20231218_SNSX_P145_1.350F466C.tar
to:
Lau_NeuroAnalytics_20241112_2024_11_12_209_8bee69a3-a558-44.25602A30.tar

The parsing in tar2bids is kludgy, so that will get a bit more awkward unless we move it to actual regex.. Not sure if the pattern (8hex-4hex-2hex?) will be consistent for different scans - @isolovey do you know?

2. the dcmstack in heudiconv chokes on the enhanced dicoms:

ERROR: Embedding failed: Exception raised while executing Node embedder.

Traceback:
	Traceback (most recent call last):
	  File "/usr/local/lib/python3.9/dist-packages/nipype/interfaces/base/core.py", line 397, in run
	    runtime = self._run_interface(runtime)
	  File "/usr/local/lib/python3.9/dist-packages/nipype/interfaces/utility/wrappers.py", line 142, in _run_interface
	    out = function_handle(**args)
	  File "<string>", line 35, in embed_dicom_and_nifti_metadata
	  File "/usr/local/lib/python3.9/dist-packages/dcmstack/dcmstack.py", line 1163, in parse_and_stack
	    results[key] = stack_group(group, warn_on_except, **stack_args)
	  File "/usr/local/lib/python3.9/dist-packages/dcmstack/dcmstack.py", line 1117, in stack_group
	    result.add_dcm(dcm, meta)
	  File "/usr/local/lib/python3.9/dist-packages/dcmstack/dcmstack.py", line 551, in add_dcm
	    nii_wrp = NiftiWrapper.from_dicom_wrapper(dw, meta)
	  File "/usr/local/lib/python3.9/dist-packages/dcmstack/dcmmeta.py", line 1561, in from_dicom_wrapper
	    result = klass(nii_img, make_empty=True)
	  File "/usr/local/lib/python3.9/dist-packages/dcmstack/dcmmeta.py", line 1280, in __init__
	    self.meta_ext.check_valid()
	  File "/usr/local/lib/python3.9/dist-packages/dcmstack/dcmmeta.py", line 306, in check_valid
	    raise InvalidExtensionError('Missing required base '
	dcmstack.dcmmeta.InvalidExtensionError: The extension is not valid: Missing required base classification time

however, it looks like using the latest heudiconv fixes this (I've confirmed that myself); looks like it might be related to having the latest nibabel version, as mentioned in this PR:
nipy/heudiconv#800

However, we are currently using a custom fork of heudiconv that has a fix for the Bruker dicoms -- @AlanKuurstra I wonder if this is still needed with the latest heudiconv/nibabel? Or do we just need to update our custom fork? I haven't looked into this yet..

3. In some of our heuristics (most importantly the default cfmm_base heuristic), we make use of the third entry of ImageType to check whether the data was corrected with gradient non-linearity correction -- this has been 'DIS2D', 'DIS3D' or 'ND', for no distortion correction. Now the field seems to be 'NONE' -- I'd like to confirm (maybe need to get on the console and do some tests with and without correction enabled) what values appear when correction is enabled. In any case, would require a minor update to this and any other heuristics that do the same to also check for NONE.

[isolovey]

@akhanf Regarding the naming of the tar file, there are two differences between the pre- and post-upgrade datasets:

• The patient name seemingly follows a different convention. 2023_12_18_SNSX_P145 vs 2024_11_12_209. Maybe SNSX_P is now dropped? Also, cfmm2tar seems to strip off the date in the former dataset, leaving only SNSX_P145. I looked at the code briefly but couldn't figure out how or where.

• The Study ID (0020,0010) attribute used to be set to 1, but now has a different format, so it is 8bee69a3-a558-44 in the post-upgrade dataset. I don't know whether this format is consistent, but if you match everything between the last _ and first ., that should be the study ID.

[isolovey]

Generally speaking, I think it's not a good idea to encode useful information in file names. The better approach is to open the DICOM files and examine their header attributes.

I realize this may not be easy to implement at this point, but if you have to make major changes anyway, that would be a good opportunity to take another look at parsing/classifying DICOM datasets based on header contents.

[akhanf]

Thanks Igor -- I agree filename parsing is not the ideal approach but not looking to refactor this code, just wanting to get it functional again, and the changes to do that are pretty minor.

Re: the date int he patient name, sorry I just copy-pasted a tarfile on our filesystem, and turns out someone had renamed it, the date indeed should still be in the filename, so thats not an issue:
Lau_NeuroAnalytics_20231218_2023_12_18_SNSX_P145_1.350F466C.tar

I've started a PR to fix the issues I brought up, this is in #45

[AlanKuurstra]

Sorry for the late reply, I’m off work right now. Regarding heudiconv, the time zone changes have been merged upstream. nipy/heudiconv#780 The additional problem of filling the metadata sidecar file with 4d and 5d dicom metadata is not resolved upstream afaik. The problem needs to be addressed in one of heudiconv’s dependencies, dcmstack: moloney/dcmstack#84 If none of our users are using the additional metadata, I would suggest using the official heudiconv repo and employing the minmeta option which will avoid the metadata bug until the changes in dcmstack have been merged. Honestly, we may want to use minmeta by default even after upstream supports n-d enhanced dicoms. Heudiconv converts dicom to nifti using dcm2niix, but if minmeta is not used it will [convert the dicom to nifti a second time with dcmstack only for the purpose of concatenating dicom metadata for the bids sidecar file](https://github.com/nipy/heudiconv/blob/17fdfb47fe8d9d422ac914c5300ebe618f263178/heudiconv/dicoms.py#L683), which is inefficient especially for some of our larger datasets. If users need more metadata than is provided with the minmeta option, we can always require them to extract it themselves using their heuristics file and defining a custom_callable function as done [here](https://github.com/khanlab/tar2bids/blob/bb1485d68041f7762cbbfef5acce0b5b45c70b44/heuristics/cfmm_bruker.py#L94-L106)

[akhanf]

The use of --minmeta wasn't making a difference for me (ie it was still choking on the Bruker dicoms), and applying the commit you had (nipy/heudiconv@def8691) to the latest heudiconv doesn't fix things either, not sure if it is the same issue or a new one though..

alik@AFI-CBS-H-1:/local/scratch$ singularity run -e ./tar2bids_enhdicom.sif -h cfmm_bruker.py -O "--minmeta" -o test_enhdicom_bruker_updated_minmeta -P '{subject}' Everling_Marmoset_20230718_Betty_20230718_01.484D54A4.tar
	Overriding heuristic file as: cfmm_bruker.py
	Using heudiconv options: --minmeta
	Overriding output dir as: test_enhdicom_bruker_updated_minmeta
  Using custom PatientName search: {subject}
  PI=Everling Study=Marmoset Date=20230718 PatientName=Betty
session not found in {subject}
Everling_Marmoset_20230718_Betty_20230718_01.484D54A4.tar -> sub-Betty
  Running: heudiconv ... 
heudiconv --minmeta --overwrite -b -d /local/scratch/Everling_Marmoset_20230718_\{subject\}_20230718_01.484D54A4.tar -o /local/scratch/test_enhdicom_bruker_updated_minmeta -f /opt/tar2bids/heuristics/cfmm_bruker.py -s Betty  | tee /local/scratch/test_enhdicom_bruker_updated_minmeta/code/tar2bids_2024-11-20_11h13m_32370/logs/heudiconv.Betty
INFO: Running heudiconv version 1.3.2+enhdicom.0 latest 1.3.2
INFO: Need to process 1 study sessions
INFO: PROCESSING STARTS: {'subject': 'Betty', 'outdir': '/local/scratch/test_enhdicom_bruker_updated_minmeta/', 'session': None}
INFO: Processing 19 dicoms
INFO: Analyzing 19 dicoms
Traceback (most recent call last):
  File "/usr/local/bin/heudiconv", line 33, in <module>
    sys.exit(load_entry_point('heudiconv==1.3.2+enhdicom.0', 'console_scripts', 'heudiconv')())
  File "/usr/local/lib/python3.9/dist-packages/heudiconv/cli/run.py", line 30, in main
    workflow(**kwargs)
  File "/usr/local/lib/python3.9/dist-packages/heudiconv/main.py", line 479, in workflow
    prep_conversion(
  File "/usr/local/lib/python3.9/dist-packages/heudiconv/convert.py", line 217, in prep_conversion
    seqinfo = group_dicoms_into_seqinfos(
  File "/usr/local/lib/python3.9/dist-packages/heudiconv/dicoms.py", line 338, in group_dicoms_into_seqinfos
    mwinfo = validate_dicom(filename, dcmfilter)
  File "/usr/local/lib/python3.9/dist-packages/heudiconv/dicoms.py", line 175, in validate_dicom
    del mw.series_signature[sig]
  File "/usr/lib/python3.9/functools.py", line 969, in __get__
    val = self.func(instance)
  File "/usr/local/lib/python3.9/dist-packages/nibabel/nicom/dicomwrappers.py", line 886, in series_signature
    signature['image_shape'] = (self.image_shape, eq)
  File "/usr/lib/python3.9/functools.py", line 969, in __get__
    val = self.func(instance)
  File "/usr/local/lib/python3.9/dist-packages/nibabel/nicom/dicomwrappers.py", line 770, in image_shape
    raise WrapperError('Non-singular index precedes the slice index')
nibabel.nicom.dicomwrappers.WrapperError: Non-singular index precedes the slice index

I have this set-up in #45 (using this heudiconv fork: https://github.com/akhanf/heudiconv).
@AlanKuurstra since you're away I think I'll merge the PR so we can get a fix for the 3T, then can make a new issue for getting the 9.4T working again..

[AlanKuurstra]

@akhanf the above error is new and different from the metadata bug which is avoided by using the minmeta option.

I opened an issue with nibabel for the new bug:
nipy/nibabel#1388