Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Couple additional XA30 fields? #606

Closed
mharms opened this issue May 10, 2022 · 16 comments
Closed

Couple additional XA30 fields? #606

mharms opened this issue May 10, 2022 · 16 comments

Comments

@mharms
Copy link
Collaborator

mharms commented May 10, 2022

Wondering if there is value in including the following additional 0018 fields that Siemens XA30 appears to now be populating:

(0018,9005) SH [*spc_314ns]                             #  10, 1 PulseSequenceName
(0018,9073) FD 349.75999999999999                       #   8, 1 AcquisitionDuration
@mharms
Copy link
Collaborator Author

mharms commented May 10, 2022

Also, I don't know the name of it, but it appears that (0021,105c) may be a useful field where XA30 is including additional imaging options. e.g., for a T1w scan with "water excitation":
(0021,105c) CS [IR\WE] # 6, 2 Unknown Tag & Data

@mharms
Copy link
Collaborator Author

mharms commented May 10, 2022

Per https://www.mathworks.com/matlabcentral/mlc-downloads/downloads/submissions/42997/versions/94/previews/dicm_dict.m/index.html?access_key=
(0021,105c) is ScanOptions. Not sure why they are no longer using the standard field of (0018,0022) for that...

@neurolabusc
Copy link
Collaborator

neurolabusc commented May 10, 2022

@mharms can I suggest you make suggestions for BIDS fields for these DICOM tags. Am I correct that you are proposing the following translation:

  • DICOM 0018,9005 is BIDS PulseSequenceName.
  • DICOM 0018,9073 is BIDS AcquisitionDuration. Direct conversion, as units for both DICOM and BIDS are seconds.
  • If 0018,0022 is absent in the DICOM, and the manufacturer is Siemens, use 0021,105c for BIDS ScanOptions.

@mharms
Copy link
Collaborator Author

mharms commented May 10, 2022

Yes, that is my proposal, and those names are consistent with their DICOM field names. Thx.

@mharms
Copy link
Collaborator Author

mharms commented May 10, 2022

BTW: By "BIDS field" name, I was assuming that you were just referring to what to name it in the sidecar json of dcm2niix itself? (The BIDS documentation has become much too complex for me to want to get involved in the process of adding them formally to BIDS).

neurolabusc added a commit that referenced this issue May 10, 2022
@neurolabusc
Copy link
Collaborator

@mharms can you test the latest development branch. It should add these to the sidecar. I am late in releasing the Spring 2022 release, so if you can provide a timely review we can generate a new stable version.

@mharms
Copy link
Collaborator Author

mharms commented May 11, 2022

Looks good. One minor suggestion would be to position PulseSequenceName just prior to the block of ScanningSequence, SequenceVariant, ScanOptions, where it more naturally fits, rather than after Manufacturer. Thanks for including!

Edit: Or maybe putting PulseSequenceName just after the block of ScanningSequence, SequenceVariant, ScanOptions would make even more sense.

@mharms
Copy link
Collaborator Author

mharms commented May 11, 2022

Forgot to mention, do you want to do anything such that the CMRR PhysioLogs are no longer converted?

@mharms
Copy link
Collaborator Author

mharms commented May 11, 2022

The stdout during conversion reports messages like the following regarding the PhysioLog files:

Skipping Spectroscopy DICOM '20220421-ST000-EssaMidb_1/MR-SE035-rfMRI_REST_AP_PhysioLog/MR-ST000-SE035-0001
.dcm'
Unable to determine slice thickness: please check voxel size

But .nii.gz and .json files (and .bval and .bvec, if related to a dMRI scan) are getting created for the PhysioLog dicoms anyway.

@neurolabusc
Copy link
Collaborator

@mharms can you test the current development branch and close this issue if it comprehensively fulfills all your suggestions. The spring 2022 release is overdue (my fault entirely), so I am eager to close all outstanding issues.

@mharms
Copy link
Collaborator Author

mharms commented May 16, 2022

The current dev version is no longer generating any files for "PhysioLog" series from my XA30 test set.
But it also is no longer generating .bval or .bvec files for legitimate dMRI series. So, the latest commit broke something important.

It is also no longer generating .bval and .bvec files for the "SBRef" series accompanying a dMRI series, although perhaps that is desirable? (Basically, I got no .bval/.bvec files at all).

Also, regarding the stdout reporting for a PhysioLog series:

Skipping Spectroscopy DICOM '20220421-ST000-EssaMidb_1/MR-SE034-rfMRI_REST_PA_PhysioLog/MR-ST000-SE034-0001.dcm'
Unable to determine slice thickness: please check voxel size
  1. Do you want to eliminate that "Unable to determine slice thickness" line?
  2. Maybe say Skipping Spectroscopy/PhysioLog DICOM ... instead, to make that message more appropriate?

@mharms
Copy link
Collaborator Author

mharms commented May 19, 2022

One other minor, unrelated thing I just noticed. Is there a character limit on the ScanOptions output?
e.g., I have a GE DICOM for which dcmdump is reporting:

(0018,0022) CS [SAT_GEMS\EDR_GEMS\MP_GEMS\EPI_GEMS\HYPERBAND_GEMS\FILTERED_GEMS\FS] # 66, 7 ScanOptions

but the resulting json is missing the final 'S':

> "ScanOptions": "SAT_GEMS\\EDR_GEMS\\MP_GEMS\\EPI_GEMS\\HYPERBAND_GEMS\\FILTERED_GEMS\\F",

@neurolabusc
Copy link
Collaborator

@mharms can you check the development release one more time.

  • dcm2niix stores most DICOM text as null-terminated strings with 66 bytes. I have promoted ScanOptions to 256 characters.
  • Should fix the bvec detection.

@mharms
Copy link
Collaborator Author

mharms commented May 23, 2022

Looks good as regards the bval/bvec generation. However, I'm still seeing truncated ScanOptions output on the GE DICOMs for which I mentioned that issue. It appears that the latest commits only increased the allowed length of ScanOptions for Siemens data, and not for all vendors more generally?

@neurolabusc
Copy link
Collaborator

Sorry, I did not have a concrete example. I created a GE DICOM with very verbose scan options and the latest commit to the development branch should support this. Please validate with your data. The example is from dcm_qa_nih with the scan options changed with dcmodify.

@mharms
Copy link
Collaborator Author

mharms commented May 24, 2022

That indeed grabbed all of the ScanOptions string on that GE data. Thanks for the changes!!

yarikoptic added a commit to neurodebian/dcm2niix that referenced this issue Apr 29, 2024
* tag 'v1.0.20220720': (65 commits)
  GE Direct field mapping (TE1/TE2) (rordenlab#617)
  GE Direct field mapping (TE1/TE2) (rordenlab#617)
  Issue 618 (rordenlab#618)
  Update notes
  Siemens XA30 ASL parameters and ImageTypeText 0021,1175
  Reset PET values for classic DICOMs (rordenlab#616)
  PostLabelDelay for XA30, FrameDuration is only for 4D datasets (rordenlab#616)
  shims are signed (rordenlab#608)
  AcquisitionVoxelSize before any interpolation or resampling within reconstruction or image processing
  Add AcquisitionVoxelSize tag for Siemens ASL (rordenlab#608)
  Store GE ShimSetting as array (rordenlab#608)
  GE sequence details (rordenlab#608)
  Philips slice timing notes
  Verbose scan options (issue 606)
  Change scanOptions
  scan options is long string, fix bvec rejection (rordenlab#606)
  Ignore non-spatial physio data (rordenlab#606)
  Flipping Y also flips sign of determinant
  Better Siemens XA support (rordenlab#606)
  Report DwellTime for Siemens XA (rordenlab#240)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants