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

BIDS sidecar field ParallelReductionFactorInPlane too large #672

Closed
lukeje opened this issue Jan 26, 2023 · 18 comments
Closed

BIDS sidecar field ParallelReductionFactorInPlane too large #672

lukeje opened this issue Jan 26, 2023 · 18 comments

Comments

@lukeje
Copy link

lukeje commented Jan 26, 2023

Describe the bug

The reported ParallelReductionFactorInPlane in the JSON sidecar file for Siemens data is the product of the in-plane and out-of-plane parallel reduction factors.

I think the problem occurs because accelFactPE (which is the source of ParallelReductionFactorInPlane for Siemens data) is read from a string which contains the total acceleration, not just the in-plane acceleration.

To reproduce

Steps to reproduce the behavior:

  1. Download the B1+_mapping DICOM dataset from here, measured on a Siemens scanner with GRAPPA 2×2 (i.e. it should give ParallelReductionFactorInPlane = 2, ParallelReductionFactorOutOfPlane = 2)
  2. Run dcm2niix on the DICOM files
  3. See that the ParallelReductionFactorInPlane is 4 in the BIDS sidecar files, rather than 2.

Expected behavior

Ideally both ParallelReductionFactorInPlane and ParallelReductionFactorOutOfPlane would be 2, rather than ParallelReductionFactorInPlane being the product of the two in the JSON sidecar files. Note that AccelFact3D in the BIDS sidecar is 2, as expected.

Note that this data does come from a "user" sequence, but based on this comment in the dcm2niix source code, this is not the only dataset where this is a problem.

Version

Tested with the current master branch:

  • Chris Rorden's dcm2niiX version v1.0.20220720 GCC9.4.0 x86-64 (64-bit Linux)

Tested with the latest development branch:

  • Chris Rorden's dcm2niiX version v1.0.20230121 GCC9.4.0 x86-64 (64-bit Linux)

Troubleshooting

Please try the following steps to resolve your issue:

It is the latest stable release.

  • If the latest stable version fails, and you are using macOS or Linux. Does the latest commit on the development branch resolve your issue?

No, the development branch did not resolve the issue.

@neurolabusc
Copy link
Collaborator

@lukeje can you communicate with your Siemens Research Collaboration manager to help out with this. The DICOM header is not internally consistent.

While the CSA header suggests ParallelReductionFactorInPlane = 2 and ParallelReductionFactorOutOfPlane = 2:

sPat.lAccelFactPE	 = 	2
sPat.lAccelFact3D	 = 	2

The private tag suggests ParallelReductionFactorInPlane = 4 and ParallelReductionFactorOutOfPlane = 1:

(0051,1011) LO [p4]

whereas the CSA would lead one to expect to see:

(0051,1011) LO [p2s2]

Perhaps this was an early E11 release when SMS was still new and not given its own acceleration flag.

In general, dcm2niix assumes DICOM headers are consistent and truthful. We can always reverse the precedence between 0051,1011 and the CSA header. However, I would want to understand the reason for the conflict to make sure there are no unintended consequences.

@lukeje
Copy link
Author

lukeje commented Jan 26, 2023

I just checked a VE12U dataset recorded with a ported version of the sequence and the problem seems to remain there too, i.e. with GRAPPA 2×2 the string in the DICOM is:

DICOM/SIEMENS CSA HEADER/ImaPATModeText: p4(string)
DICOM/SIEMENS CSA HEADER/PATModeText: p4(string)

This doesn't rule out that Siemens product sequences might have fixed the inconsistency in VE12, though; I'll check when I'm next at the scanner.

@lukeje
Copy link
Author

lukeje commented Jan 27, 2023

Looking again at the dcm2niix source code, it looks like even if the out-of-plane GRAPPA acceleration factor was stored as M in the CSA string pNsM, it would be assumed to be the (2D) multiband factor, rather than the (3D) out-of-plane acceleration factor. Once I've got an example dataset from a product sequence I can try and reach out to someone at Siemens to see what interpretation of this string is correct (assuming you don't already have a comprehensive answer).

@neurolabusc
Copy link
Collaborator

@lukeje please also take a look at the validation series DKI_b2000_20_2p2mm_iPAT where the CSA slice timing clearly demonstrates a multi-band of two.

This is clearly noted in the tag 0051,1011:

(0018,1020) LO [syngo MR E11]            #SoftwareVersions
(0018,1030) LO [DKI_b2000_20_2p2mm_iPAT] #ProtocolName
(0051,1011) LO [p2 s2]                   #Private Tag & Data

However, the explicit SMS tag in the CSA header incorrectly suggests that the MultibandAccelerationFactor is 1:

sPat.lAccelFactPE	 = 	2
sPat.lAccelFact3D	 = 	1

This dataset appears to provide the opposite interpretation to yours, suggesting that precedence should be given to 0051,1011. Perhaps this reflects the custom CMRR sequences (ConsistencyInfo: N4_VE11C_LATEST_20160120), but it would be great to have an algorithm that works reliably for both product and research sequences.

@neurolabusc
Copy link
Collaborator

@lukeje perhaps the issue here is the nomenclature for SMS acceleration of 2D EPI scans and 3D sequences. The BIDS specification clearly defines MultibandAccelerationFactor for EPI scans, but does not (yet) define ParallelReductionFactorOutOfPlane that would be an analog of DICOM 0018,9155. dcm2niix already defines a lot of DICOM tags that are not in the BIDS standard, so I am happy to add this while we lobby BIDS to include this one-to-one mapping.

The main thing I would want is some clarity regarding when we use the field MultibandAccelerationFactor and when we use ParallelReductionFactorOutOfPlane, along with validation datasets.

@mharms may have some thoughts on this.

@lukeje
Copy link
Author

lukeje commented Jan 27, 2023

Sorry, I thought you knew that ParallelReductionFactorOutOfPlane is in the latest BIDS specification (see discusion here).

@neurolabusc
Copy link
Collaborator

The creators of the Example dataset for the hMRI toolbox should be admonished, as this provides an example of desired output without a reference input dataset. I am glad BIDS now explicitly encourages sourcedata files. There has been a rampant creation of new BIDS fields that represent a wish list of desired sequence information with no consideration of how the user will determine this information, describing how different manufacturers use these terms, and validation datasets for regression detection. The opportunity for unintended consequences is profound. I worry that new BIDS fields are created by experts in sub-domains, but the burden of implementing them is pushed to users who do not have the expertise to decipher what is desired.

Perhaps your earlier link to a DICOM dataset is an attempt to rectify this situation.

If you want these fields to be supported, I strongly encourage following the format of the dcm_qa series that provides explicit examples for usage such as slice timing, enhanced DICOM, and gantry tilt, and ASL parameters. These provide a one-to-one mapping from source DICOM images to expected BIDS results. Each commit of dcm2niix is tested with a battery of these validation datasets to avoid regressions, and this allows us to harmonize our use with other tools such as dicm2nii. The other advantage of these datasets is that they help identify gaps in the current DICOM specification, which allows us to work with manufacturers and the DICOM work groups to aid reproducible science.

@mharms
Copy link
Collaborator

mharms commented Jan 27, 2023

@lukeje please also take a look at the validation series DKI_b2000_20_2p2mm_iPAT where the CSA slice timing clearly demonstrates a multi-band of two.

This is clearly noted in the tag 0051,1011:

(0018,1020) LO [syngo MR E11]            #SoftwareVersions
(0018,1030) LO [DKI_b2000_20_2p2mm_iPAT] #ProtocolName
(0051,1011) LO [p2 s2]                   #Private Tag & Data

However, the explicit SMS tag in the CSA header incorrectly suggests that the MultibandAccelerationFactor is 1:

sPat.lAccelFactPE	 = 	2
sPat.lAccelFact3D	 = 	1

This dataset appears to provide the opposite interpretation to yours, suggesting that precedence should be given to 0051,1011. Perhaps this reflects the custom CMRR sequences (ConsistencyInfo: N4_VE11C_LATEST_20160120), but it would be great to have an algorithm that works reliably for both product and research sequences.

@neurolabusc : I think there is a confusion in your earlier post. I'm pretty sure that sPat.lAccelFact3D represents what is being called ParallelReductionFactorOutOfPlane, not the multi-band factor.
So, the validation series you were referencing would have GRAPPA = 2 x 1, with MB factor = 2.

@neurolabusc
Copy link
Collaborator

neurolabusc commented Jan 27, 2023

@mharms yes, I realized that when I wrote my comment regarding 2D vs 3D data. So the s value of (0051,1011) encodes the product of sPat.lAccelFactPE and sPat.lAccelFact3D. This would provide some rationale for giving precedence to the CSA header when it is available.

This suggests we extend the V* conversion:

  • The CSA series header (0029,1020) tag sSliceAcceleration.lMultiBandFactor is the V* analog for the BIDS field MultibandAccelerationFactor.
  • The CSA series header (0029,1020) tag sPat.lAccelFact3D is the V* analog for ParallelReductionFactorOutOfPlane.

To be comfortable adding support for this, I would really like to have a concrete example where ParallelReductionFactorOutOfPlane and ParallelReductionFactorInPlane are modulated to document and replicate this interpretation. Since support for Windows 7 has now ended, it would be great to see concrete examples for V*, XA interoperability/classic and XA enhanced.

@mharms
Copy link
Collaborator

mharms commented Jan 27, 2023

Isn't it the p (not s) value of (0051,1011) that is giving the product of the two GRAPPA accelerations?

So, if I'm inferring correctly, ParallelReductionFactorInPlane is currently taken from the p value of (0051,1011), not from sPat.lAccelFactPE ?

@neurolabusc
Copy link
Collaborator

neurolabusc commented Jan 27, 2023

@mharms, yes. And if we are lucky with XA ParallelReductionFactorOutOfPlane is reported by DICOM Tag 0018, 9155.

@mharms
Copy link
Collaborator

mharms commented Jan 27, 2023

I see that it's reported in some structural scans I have as Enhanced DICOMs from XA30:

    (0018,9069) FD 2                                        #   8, 1 ParallelReductionFactorInPlane
    (0018,9077) CS [YES]                                    #   4, 1 ParallelAcquisition
    (0018,9078) CS [GRAPPA]                                 #   6, 1 ParallelAcquisitionTechnique
    (0018,9079) FD 1000                                     #   8, 1 InversionTimes
    (0018,9081) CS [NO]                                     #   2, 1 PartialFourier
    (0018,9155) FD 1                                        #   8, 1 ParallelReductionFactorOutOfPlane

None of these (0018,9***) field are included in Classic DICOMs exported from the same scanner.

I don't know if it adds anything, but there is a sPat.dTotalAccelFact field, which I assume is just the product of sPat.lAccelFactPE and sPat.lAccelFact3D.

@lukeje
Copy link
Author

lukeje commented Jan 27, 2023

Hey Chris, I would appreciate it if you stopped tilting at windmills. I have made a targeted bug report and would appreciate it if you remained on topic. As has become clear, the current behaviour of dcm2niix does not match what would be expected.

You are correct that the dataset I am currently converting is related to me trying to prepare a dataset for you in relation to a previous bug report. However the extra fields that were requested there are not relevant here. Indeed, I hope that we (and other members of the hMRI toolbox development team) can work together to include the extra fields from the qMRI "wishlist" in future, after further communication.

@neurolabusc
Copy link
Collaborator

neurolabusc commented Jan 29, 2023

@lukeje I only have a single exemplar, so I would be grateful if can you test out the latest commit to the development branch (v1.0.20230127). It uses the new BIDS tag ParallelReductionFactorOutOfPlane instead of the previous AccelFact3D (if from the CSA) or ParallelReductionOutOfPlane (if from 0018, 9155 or 0043,1083). The CSA values are given precedence over (0051,1011).

You can get a pre-compiled copy by selecting the desired operating system and then artifact from AppVeyor, or build it yourself with UNIX:

git clone --branch development https://github.com/rordenlab/dcm2niix.git
cd dcm2niix/console
make

@neurolabusc
Copy link
Collaborator

@mr-jaemin can you please test the behavior of the latest commit. I assume that I use the reciprocal of the second item in the 0043,1083 for ParallelReductionFactorOutOfPlane. This is related to your help with issue 641.

The exemplar I have is the T1 scan from dcm_qa_ge which reports

(0043,1083) DS [0.5\0.5]

which I assume converts to BIDS:

"ParallelReductionFactorInPlane": 2,
"ParallelReductionFactorOutOfPlane": 2,

@mr-jaemin
Copy link
Collaborator

mr-jaemin commented Feb 1, 2023

I collected different combinations of ParallelReductionFactorInPlane (Acceleration-> Phase) ParallelReductionFactorOutOfPlane (Acceleration-> Slice) on GE as shown below
Screen Shot 2023-02-01 at 9 20 03 AM

For completeness, I also collected a dataset with Acceleration -> HyperSense (Compressed Sensing) (although this is off the topic for this particular issue)
Screen Shot 2023-02-01 at 9 27 05 AM

Here is the list of scans, where P: Phase, S: Slice, H: HyperSense
Screen Shot 2023-02-01 at 9 40 01 AM

I confirmed that everything is working as expected.
For example, 09_T1_MPRAGE_P1.2S1.44 reports

(0043,1083) DS [0.833333\0.694444]                      #  18, 2 AssetRFactors

which converts to BIDS

	"ParallelReductionFactorInPlane": 1.2,
	"ParallelReductionFactorOutOfPlane": 1.44,

The stable release of dcm2niix (v1.0.20220915), as expected, reports Acceleration-> Slice as ParallelReductionOutOfPlane, not ParallelReductionFactorOutOfPlane

	"ParallelReductionFactorInPlane": 1.2,
	"ParallelReductionOutOfPlane": 1.44,

Perhaps, I will discuss with Chris separately about HyperSense's Acceleration factor (as shown below) and how to share these exemplars publicly.

(0043,10b7) LO [1.24\1\10\0]                            #  12, 4 Compressed Sensing Parameters

@neurolabusc
Copy link
Collaborator

@mr-jaemin thanks. Closing this issue as it passes all samples provided. I will deal with compressed sensing and deep learning in a future issue when I have samples from other vendors.

@lukeje
Copy link
Author

lukeje commented Feb 12, 2023

I would be grateful if can you test out the latest commit to the development branch (v1.0.20230127).

Sorry for the late reply; yes, the new development branch works as expected.

Thanks a lot!

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

4 participants