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] Update AcquisitionDuration definition to match DICOM, define FrameAcquisitionDuration for sparse sequences #1974

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

Conversation

effigies
Copy link
Collaborator

@effigies effigies commented Nov 1, 2024

This PR brings the AcquisitionDuration definition in line with DICOM and separates it from checks for sparse sequence descriptions. We already narrowed the AcquisitionDuration mutual exclusion rules to only apply to BOLD and ASL data.

This defines FrameAcquisitionDuration, which corresponds to DICOM tag 0018, 9220.

@neurolabusc I wonder if this tag is used in any of your QA datasets yet?
@Lestropie Are you okay with deferring a CT-applicable definition to when CT data is accepted in BIDS? As noted in #1906 (comment), we do have mechanisms in the schema to provide multiple definitions for terms that are used in multiple contexts.

Closes #1906.


I would also like to relax the mutual exclusion constraints that are indicated in Task Imaging Data - Timing Parameters:

image

Given that RepetitionTime, SliceTiming and FrameAcquisitionDuration could all be present in the DICOM metadata, I think a better check would be for consistency among these, instead of enforcing mutual exclusion. DelayTime and VolumeTiming cannot be derived from DICOM tags (I don't think), so I would be okay with continuing to validate mutual exclusion or to validate consistency.

RepetitionTime and VolumeTiming need to remain mutually exclusive, as they are distinct ways of declaring the onsets of each volume.

I am happy to enact this change in this PR or another, if people agree that it's worth doing.

@effigies effigies requested a review from erdalkaraca as a code owner November 1, 2024 15:48
@Lestropie
Copy link
Collaborator

Are you okay with deferring a CT-applicable definition to when CT data is accepted in BIDS?

I've no issue proceeding with a definition that's not perfect for CT if there is not yet CT support in BIDS. I only mentioned it in #1906 to show that the inconsistency in definition of this parameter is not exclusively between DICOM and BIDS, but even within DICOM itself.

@neurolabusc
Copy link

I think this is fine. As the BIDS specifcation notes for SliceTiming Without this parameter slice time correction will not be possible, so that provides more granularity than the other options. I do wonder if we should include a graphic or at lest a link to the DICOM Figure C.7.6.16-2. Hopefully the fence post problem is clear with the existing definitions, e.g. the final slice time is the onset time for the final slice in a 3D volume relative to the onset time of the first slice in the volume. In contrast, Acquisition Duration spans the start of the first temporal slice to the completion of the final slice.

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

Successfully merging this pull request may close these issues.

[BUG] Inconsistent definition of AcquisitionDuration
3 participants