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

[ENH] Add generic metadata from BEP22 to MRI #1396

Merged
merged 9 commits into from
Apr 11, 2024

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Feb 5, 2023

Aim

Adds metadata to MRI and PET that either already exist in other modalities (BodyPart*) or are part of BEP22 (MRS).

See comment #1377 (comment)

This should:

  • help make the MRS BEP is smaller by integrating a chunck of it early
  • try to maximize metadata consistency between modalities

@codecov
Copy link

codecov bot commented Feb 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.93%. Comparing base (83b12e9) to head (f3dd861).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1396   +/-   ##
=======================================
  Coverage   87.93%   87.93%           
=======================================
  Files          16       16           
  Lines        1351     1351           
=======================================
  Hits         1188     1188           
  Misses        163      163           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Feb 5, 2023

I am not entirely sure if some of those have existing DICOM tags for MRI data.

BodyPart for PET mentions "DICOM Tag 0018, 0015 Body Part Examined", but I am not sure how it is for MRI data.

@neurolabusc if you have any hints on the DICOM fields for those metadata it'd be much appreciated.

src/schema/rules/sidecars/mri.yaml Outdated Show resolved Hide resolved
src/schema/rules/sidecars/mri.yaml Outdated Show resolved Hide resolved
src/schema/rules/sidecars/mri.yaml Outdated Show resolved Hide resolved
src/schema/objects/metadata.yaml Outdated Show resolved Hide resolved
@neurolabusc
Copy link

The DICOM Body Part Examined Attribute (0018,0015) is optional (Type 3). It is often included in MRI scanners with most vendors using BRAIN, but sometimes HEAD for Neuroimaging scans. If you need sourcedata/ for this, you could include the public data from dcm_qa_stc where GE and Siemens report (0018,0015) as [BRAIN] while UIH reports [HEAD].

@neurolabusc
Copy link

@Remi-Gau do you have source data exemplars for proposed tags like WaterSuppression, WaterSuppressionTechnique, B0ShimmingTechnique, B1ShimmingTechnique, and NumberReceiveCoilActiveElements? I do not think these have DICOM tags, so perhaps it is best to propose to the DICOM workgroup rather than BIDS.

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Feb 7, 2023

@Remi-Gau do you have source data exemplars for proposed tags like WaterSuppression, WaterSuppressionTechnique, B0ShimmingTechnique, B1ShimmingTechnique, and NumberReceiveCoilActiveElements? I do not think these have DICOM tags, so perhaps it is best to propose to the DICOM workgroup rather than BIDS.

I actually do not.

Those are metadata being added to the BIDS MRS extension (see for example here:

)

I am just trying to also add them to the MRI datatypes to help have more internal consistency within BIDS.

Random question: would there be an issue in adding those to BIDS AND DICOM ?

@neurolabusc
Copy link

@Remi-Gau your question is timely. The DICOM Executive Committee is encouraging the collaboration between the BIDS steering committee and the DICOM Magnetic Resonance Work Group (WG-16). The Work Groups have representatives from all the manufacturers. While things take time to trickle through, it is an opportunity to create a virtuous loop between what users need and how different manufacturers refer to items.

@effigies
Copy link
Collaborator

@Remi-Gau How about we break out the sample components into a separate PR from the acquisition parameters? This will complement #1586.

@Remi-Gau
Copy link
Collaborator Author

@Remi-Gau How about we break out the sample components into a separate PR from the acquisition parameters? This will complement #1586.

oh yes!!! that would be nice

wanna open a PR or should I?

@effigies
Copy link
Collaborator

Go for it, or I can tomorrow.

@effigies
Copy link
Collaborator

Opened #1593.

@effigies effigies force-pushed the extra_mri_metadata branch from 8c6f280 to 3664403 Compare August 24, 2023 01:45
@effigies effigies changed the title [ENH] add extra mri / pet metadata [ENH] Add generic metadata from BEP22 to MRI Aug 24, 2023
@Remi-Gau
Copy link
Collaborator Author

removed sampled metadata as this is taken care of in #1593

@effigies
Copy link
Collaborator

Ah, I just rebased this on top of #1593. If you want it separate, just rebase 3664403 on master?

@sappelhoff sappelhoff added the BEP label Nov 30, 2023
@Remi-Gau Remi-Gau added the MRI For things that affect all MRI datatypes label Dec 22, 2023
@effigies
Copy link
Collaborator

@markmikkelsen Trying to get a handle on this. Do we have example DICOMs that generate these additional terms? How do MRS converters determine them?

Comment on lines +72 to +73
WaterSuppression: optional
WaterSuppressionTechnique: optional
Copy link
Contributor

@markmikkelsen markmikkelsen Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@effigies DICOM tag (0018,9025) fits these, but the value is not boolean as is proposed in #1377. The DICOM tag also covers OuterVolumeSuppression that the MRS BEP includes (which would be the FAT value in DICOM).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So do we want to add something like the following to WaterSuppression:

Corresponds to DICOM Tag 0018, 9025 SpectrallySelectedSuppression.
If present and not NONE, then this value SHOULD be True, otherwise False.

And the following to WaterSuppressionTechnique?

Corresponds to DICOM Tag 0018, 9025 SpectrallySelectedSuppression.
May be omitted if the value is NONE.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! And the same would be the case for OuterVolumeSuppression.

@markmikkelsen
Copy link
Contributor

I want to point out that most MR manufacturers don't even use the DICOM format to export/store MRS data. They use their own format. Hence why we designed the NIfTI-MRS format.

If MRS datasets are "DICOM" formatted, manufacturers alter them to their own specifications. So, I feel conforming MRS metadata in BIDS to DICOM is limiting what MRS users actually need for sharing data in BIDS format.

Is there a reason why BIDS should conform so much to DICOM?

@neurolabusc
Copy link

Perhaps @wtclarke can provide insight.

@markmikkelsen while BIDS is distinct from DICOM, it is useful to use the same terminology when it is already established in one standard. Furthermore, the BIDS steering group is actively collaborating with the relevant DICOM working groups (composed of engineers from the manufacturers) to establish common terminology where none exists. While this does slow down the process, the aim is for manufacturers to move from private DICOM tags or private manufacturer data formats to public DICOM tags. So while current MRS data may not use DICOM or public tags, the goal is in the future they will be able to. It makes sense to generate new terms when the existing ones are ambiguous or where manufacturers use competing terms. However, it makes sense to involve the manufacturers in the dialog to harmonize our terminology.

@markmikkelsen
Copy link
Contributor

Thanks, @neurolabusc—that's very helpful to know! I'm glad the manufacturers are working together to better harmonize terminology. As you know, it's been a particularly challenging issue for MRS software developers.

@Remi-Gau Remi-Gau added this to the 1.10.0 milestone Mar 23, 2024
@markmikkelsen
Copy link
Contributor

Coming back to this from the MRS-BEP:

B0ShimmingTechnique
NumberReceiveCoilActiveElements

As far as I am aware, these have no equivalent DICOM tags but are very useful to the understanding of measurement of outcomes data outcomes of MRS users (e.g., see our multi-site paper)

B1ShimmingTechnique
NumberTransmitCoilActiveElements

These are relevant to parallel RF transmission in MRI and MRS, but they do not seem to have any DICOM tags.

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a domain expert, but as far as I can tell this looks reasonable to me. Thanks!

@Remi-Gau Remi-Gau merged commit 591eebb into bids-standard:master Apr 11, 2024
26 checks passed
@Remi-Gau Remi-Gau deleted the extra_mri_metadata branch April 11, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BEP MRI For things that affect all MRI datatypes
Projects
Development

Successfully merging this pull request may close these issues.

5 participants