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

[SCHEMA] physio and stim not included for EEG, MEG, iEEG and PET #1412

Closed
6 of 13 tasks
Remi-Gau opened this issue Oct 3, 2021 · 13 comments · Fixed by bids-standard/bids-specification#894
Closed
6 of 13 tasks
Labels

Comments

@Remi-Gau
Copy link
Contributor

Remi-Gau commented Oct 3, 2021

physio and stim suffixes are currently included for func and beh.

But they are not included for the following data types:

Implement validation for validator:

  • EEG
  • iEEG
  • MEG
  • PET
  • ASL
  • DWI
  • mri anat

I can't think of a reason for why that's the case, should we add those to the schema ?

@Remi-Gau Remi-Gau added the schema label Oct 3, 2021
@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 3, 2021

pinging @tsalo 😉

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 3, 2021

actually the physio page says this:

For the template directory name, <datatype> can correspond to any data recording modality, for example func, anat, dwi, meg, eeg, ieeg, or beh.

So I suspect that PET should not be included but some other datatypes should be included.

This might make make the templates of some section complicated to read: for example the one for anat.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 3, 2021

On this topic: @mnoergaard @melanieganz

should PET support physio / stim recording ?

Or is it like supporting task for PET? See this comment: bids-standard/bids-specification#804 (comment)

@effigies
Copy link
Collaborator

effigies commented Oct 3, 2021

It seems like it should. Should also check in about ASL.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 3, 2021

@HenkMutsaerts @patsycle

@HenkMutsaerts
Copy link

That should be fine,as this doesn't significantly change the ASL specification, as far as i understand now?

We decided however not to support functional ASL yet as this would require all non task ASL (99% of ASL data) to include task-rest. @patsycle was there also another reason?

Physiological simuli are more common in ASL, in the form of an acetazolamide challenge, breath hold, or CO2 challenge. Would this be covered by physio/stim?

@patsycle
Copy link
Contributor

patsycle commented Oct 4, 2021

@HenkMutsaerts @Remi-Gau Indeed, functional ASL might get implemented later on. If I understand correctly, it was now mainly an issue to get it working in the validator. Marco might remember this.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 4, 2021

Physiological simuli are more common in ASL, in the form of an acetazolamide challenge, breath hold, or CO2 challenge. Would this be covered by physio/stim?

yeah this would be one of the most straightforward way of gas concentration and respiration during your acquisition and it does not need to change anything to the part of ASL spec. We just need mention the physion / stim page that ASL supports it.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 4, 2021

@HenkMutsaerts @Remi-Gau Indeed, functional ASL might get implemented later on. If I understand correctly, it was now mainly an issue to get it working in the validator. Marco might remember this.

If you are referring to the fact that physio files in the perf folder should also be also be validated that is correct. This would need to be implemented.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 4, 2021

I suggest implementing this in several PRs split up by datatypes with a counterparts on the validator side.

  • eeg, meg, ieeg
  • dwi
  • asl
  • pet
  • anat (I am struggling to imagine the use case for physio and stim for anat but I may be missing something)

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Oct 5, 2021

OK I will wait for the bids-standard/bids-specification#883 to be in before opening PRs on this to avoid potential merge conflicts.

@Remi-Gau
Copy link
Contributor Author

Remi-Gau commented Feb 1, 2022

Will actually transfer this to the validator to remember to implement it.

@Remi-Gau Remi-Gau reopened this Feb 1, 2022
@Remi-Gau Remi-Gau transferred this issue from bids-standard/bids-specification Feb 1, 2022
@effigies
Copy link
Collaborator

effigies commented Aug 1, 2024

I think this is done. If it's not, please open a new issue with a clearer requirement.

@effigies effigies closed this as completed Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants