Skip to content

Allow direction entity in MESE files #2100

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Apr 14, 2025

Closes none.

Changes proposed:

  • Split MESE and MEGRE suffix entity sets.
  • Add optional direction entity to the raw MESE anatomical suffix.

@tsalo tsalo added structural MRI MRI For things that affect all MRI datatypes labels Apr 14, 2025
@tsalo tsalo requested a review from erdalkaraca as a code owner April 14, 2025 13:29
Copy link

codecov bot commented Apr 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.65%. Comparing base (a7dd34a) to head (b0909fb).
Report is 8 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2100   +/-   ##
=======================================
  Coverage   82.65%   82.65%           
=======================================
  Files          17       17           
  Lines        1534     1534           
=======================================
  Hits         1268     1268           
  Misses        266      266           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tsalo tsalo requested review from effigies and ericearl April 14, 2025 13:49
@effigies
Copy link
Collaborator

What's the use case?

@tsalo
Copy link
Member Author

tsalo commented Apr 14, 2025

I have AP and PA MESE scans in a current study, but I don't know why. I don't know if there's a use-case as much as it is something researchers can control, so it seems like it should be reflected in the entity set.

@tsalo
Copy link
Member Author

tsalo commented Apr 14, 2025

Apparently we plan to use the AP and PA MESE scans for distortion correction.

@oesteban
Copy link
Collaborator

In anatomical images, I think this would be an acceptable use of acq-, but I'd be okay with this change in general.

Apparently we plan to use the AP and PA MESE scans for distortion correction.

Please make sure there's nothing in the B0FieldIdentifier/B0FieldSources interface that disallows it from setting identifiers on these, so distortion can be estimated.

@effigies
Copy link
Collaborator

Okay, so you have a use case for AP/PA MESE scans, but what about all the other groups? Do you get AP/PA T1w scans, or is this more in-passing to avoid doing this piecemeal, if other use cases are found?

At least for parametric volumes, I would assume that directions would be inapplicable by the time the final volumes are generated, similar to how echo, flip and part no longer apply.

I am hesitant to add more to the nonparametric group without a specific use case.

@oesteban
Copy link
Collaborator

Okay, so you have a use case for AP/PA MESE scans, but what about all the other groups?

If the only use case is fieldmap estimation, then acq- plus ensuring B0FieldIdentifier is explicitly allowed in their metadata should be sufficient to encode this.

@yarikoptic
Copy link
Collaborator

If there is operator ability to specify direction in anatomicals, we should allow for this entity. IMHO, overall, when there is appropriate targeted entity, we better allow for it instead of inspiring to absorb into some more generic one (like _acq).

@effigies
Copy link
Collaborator

I agree that we shouldn't overload acq, and am happy to approve this for MESE. My hesitation is over a lack of use-case for other suffixes.

@tsalo
Copy link
Member Author

tsalo commented Apr 25, 2025

Okay, I've limited my change to the MESE and MEGRE suffixes. I don't have any multi-PED MEGRE data, but I figured it was easier to just add direction to that one entity set than to split the rule.

@tsalo tsalo changed the title Allow direction entity in anatomical files Allow direction entity in MESE and MEGRE files Apr 25, 2025
@effigies
Copy link
Collaborator

I would split it. I'm sure that the only reason we kept them together was because they had the same entities at the time, not because of some fundamental sameness.

I could be exposing my ignorance to the world, but my understanding is that gradient echo sequences just don't have the same susceptibility distortions that would justify paired direction acquisitions.

@tsalo tsalo changed the title Allow direction entity in MESE and MEGRE files Allow direction entity in MESE files Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MRI For things that affect all MRI datatypes needs review structural MRI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants