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

[BUG] Entity requirements for MPM suffix do not appear correct #1683

Closed
lbanellis opened this issue Jan 23, 2024 · 13 comments
Closed

[BUG] Entity requirements for MPM suffix do not appear correct #1683

lbanellis opened this issue Jan 23, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@lbanellis
Copy link

Describe your problem in detail.

page in the spec https://bids-specification.readthedocs.io/en/stable/appendices/file-collections.html
we tested with @melanieganz @CPernet @Remi-Gau MPM it is valid only if entities flip and mt are present

  • issue 1: in the table it does not explain what are the required entities
  • issue 2: in our case we use multiple echo times and single flip, makes no sense one entity over others matter more

Describe what you expected.

No response

BIDS specification section

https://bids-specification.readthedocs.io/en/latest/...

@lbanellis lbanellis added the bug Something isn't working label Jan 23, 2024
@lbanellis
Copy link
Author

issue one can be fix right now, then we can disccuss issue 2 how to solve it
@agahkarakuzu

@Remi-Gau
Copy link
Collaborator

Relevant schema section:

vfamt:
suffixes:
- MPM
- MTS
extensions:
- .nii.gz
- .nii
- .json
datatypes:
- anat
entities:
subject: required
session: optional
task: optional
acquisition: optional
ceagent: optional
reconstruction: optional
run: optional
echo: optional
flip: required
mtransfer: required
part: optional
chunk: optional

@agahkarakuzu
Copy link
Contributor

@lbanellis the entity table explains required entities. As well as the suffix:
image

issue 2: in our case we use multiple echo times and single flip, makes no sense one entity over others matter more

It makes sense in the context of an MPM acquisition, which is why it exists as a file collection. For example, if you are not varying your FAs, how do you get an R1 map? If you are not, is that an MPM?

@CPernet
Copy link
Collaborator

CPernet commented Jan 23, 2024

@agahkarakuzu what we meant is it makes no sense the echo is not mandatory and flip is, given that here the parameters that matters is echo time, ie what is mandatory or optional is weird (fixed echo = don't mention it but of course you have multiple flips, conversely fix flip = required to write flip-1 for the different echos)

@CPernet
Copy link
Collaborator

CPernet commented Jan 23, 2024

we have to do

sub-X_acq_PWw_echo-1_flip-1_mt-off_MPM.ext  
sub-X_acq_PWw_echo-2_flip-1_mt-off_MPM.ext  
sub-X_acq_PWw_echo-3_flip-1_mt-off_MPM.ext  

while

sub-X_acq_PWw_flip-1_mt-off_MPM.ext  
sub-X_acq_PWw_flip-2_mt-off_MPM.ext  
sub-X_acq_PWw_flip-3_mt-off_MPM.ext  

is fine

@CPernet
Copy link
Collaborator

CPernet commented Jan 23, 2024

issue one is to have the info in the table

@lbanellis
Copy link
Author

Also, noticed the BIDS validator is very sensitive to the order of the entities (is only happy when echo and run is after acq)

@Remi-Gau
Copy link
Collaborator

Also, noticed the BIDS validator is very sensitive to the order of the entities (is only happy when echo and run is after acq)

That's expected: entities can only appear in a specific order.

The order when taking ALL entities into account is the one you can see in this document:
https://bids-specification.readthedocs.io/en/latest/appendices/entities.html

More succintly put in the schema:
https://github.com/bids-standard/bids-specification/blob/3bf993b1d5d40e87375f554f0214d321e9696677/src/schema/rules/entities.yaml

@lbanellis
Copy link
Author

Thank you for your replies, this issue was written with the organisers of the BIDS BrainHack event as it took us some time to convert the MPMs to validated BIDS with the organisers help.

@Remi-Gau
Copy link
Collaborator

@lbanellis

Was pointed out to me that sending links to yml files is not the most user friendly. Sorry about that.

The way you usually know how to name files is to check the filename templates that are generated for each datatype.

The one with MPM should be right there:

https://bids-specification.readthedocs.io/en/latest/appendices/file-collections.html#magnetic-resonance-imaging

image

This will tell you for a given suffix what are the entities you can use, which ones are required (see the legend dropdown below the templates for more info on that) and in which order they should appear.

All filenames templates of all datatypes are also listed in a single page on the bids starter kit

https://bids-standard.github.io/bids-starter-kit/folders_and_files/files.html#anat-anatomical-mri

@CPernet
Copy link
Collaborator

CPernet commented Jan 26, 2024

done a PR just to move info in the main spec -- but I still think it makes no sense for MPM which entities are mandatory vs optional

  • case 1, one acquires data using variable flip angles and one stores the various flips but not the inversion time (in the json a single value, makes sense)
  • case 2, one acquires data using multiple inversion times and have the inversion time becomes inherently needed (while optional) to distinguish files but have to store the same flip many times (while being a single value, makes no sense)

@tsalo
Copy link
Member

tsalo commented Feb 15, 2024

@CPernet the definition for "MPM" that we have in the specification is consistent with the entity requirements in the schema. If there is a canonical definition for the MPM protocol that differs from how we define it, and makes it clear what the requirements are, then can you link to that and we can update the definition?

@tsalo tsalo changed the title [BUG] file collection entities issue [BUG] Entity requirements for MPM suffix do not appear correct Feb 15, 2024
@CPernet
Copy link
Collaborator

CPernet commented Mar 21, 2024

the entity requirement are 'correct' code wise, I am arguing here for reference that scientifically speaking this makes little sense to stores the various flips (mandatory) while the inversion time (optional) if using multiple flip angles data acquisition scheme (which is fine) but have to use the optional inversion time and store the same flip value many times when using a multiple inversion data acquisition scheme (ie either both mandatory or have some conditional if multiple flip, inversion is optional because always the same, if multiple inversion, flip is optional because always the same)

@CPernet CPernet closed this as completed Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants