-
Notifications
You must be signed in to change notification settings - Fork 161
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] BEP001 - New entities: inv & mt #681
Conversation
Pull upstream changes
Update master
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just commenting on the markdown structure: the table pipes on lines 55 to 60 will have to be aligned
src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md
Outdated
Show resolved
Hide resolved
…ata.md Co-authored-by: Stefan Appelhoff <[email protected]>
Co-authored-by: Stefan Appelhoff <[email protected]>
I still cannot push to this PR (same issue as in some previous PR) :-) @agahkarakuzu can you please give me "maintain" or "admin" rights for the bids-bep001 organization: https://github.com/bids-bep001 ? |
@sappelhoff unfortunately I can't because I am not the owner :/ if this need base to master, I can do that again. |
no, I just took a minute to align the table pipes and was hoping that I could push that change to the PR, but I can't :( so I think you'll have to do it yourself. Or paste this below line 45:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. One thought:
src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md
Outdated
Show resolved
Hide resolved
@agahkarakuzu then who is the owner of that organization? There are no "public" members, so I cannot check myself. see: https://github.com/orgs/bids-bep001/people It'd be good to give BIDS maintainers admin or at least maintainer rights for that organization. Otherwise we cannot make edits to PRs that are done from branches owned by the bids-bep001 organization. cc @Gilles86 |
@KirstieJane is the sole owner of the bid-bep001 Github organization. Maybe she can help? |
This is also a bit on me, the initial idea was to merge our PRs to the bep001 branch on bids-organization (what we did in the first big PR and @KirstieJane gave me write access to it).However, splitting PRs and their co-occurrence made that approach a bit uneasy. If @KirstieJane sees fit, adding a BIDS maintainer to the |
* [DOC] Auto-generate changelog entry for PR #677 * [ENH] BEP001 - New entities: inv & mt (#681) * add entity inv * add mt: * add MT specific metadata * Generate entity.md * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Stefan Appelhoff <[email protected]> * Update src/schema/entities.yaml Co-authored-by: Stefan Appelhoff <[email protected]> * table fix by @sappelhoff * Conditional requirement Co-authored-by: Stefan Appelhoff <[email protected]> * [ENH] BEP001 - Entity-linked file collections (#688) * Squashed commit of the following: commit 8e1ac5f Author: Stefan Appelhoff <[email protected]> Date: Fri Nov 27 11:09:24 2020 +0100 fix links and latin commit 27c9b41 Author: Stefan Appelhoff <[email protected]> Date: Fri Nov 27 10:03:51 2020 +0100 remove latin Co-authored-by: Chris Markiewicz <[email protected]> commit 7d87070 Author: Agah <[email protected]> Date: Mon Nov 23 08:46:47 2020 -0500 Update src/99-appendices/10-file-collections.md Co-authored-by: Chris Markiewicz <[email protected]> commit b137a9e Author: Agah <[email protected]> Date: Mon Nov 23 08:45:19 2020 -0500 Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> commit d7897b8 Author: Agah <[email protected]> Date: Mon Nov 23 08:44:40 2020 -0500 Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> commit 09e9d32 Author: Agah <[email protected]> Date: Mon Nov 23 08:44:15 2020 -0500 Commit suggestion Co-authored-by: Chris Markiewicz <[email protected]> commit 009a9ab Author: Agah Karakuzu <[email protected]> Date: Mon Nov 23 08:43:00 2020 -0500 Add file-collecitons appendix to the TOC commit 8f6c4df Author: Agah Karakuzu <[email protected]> Date: Mon Nov 23 08:41:13 2020 -0500 Address suggestion by @tsalo commit 45210a9 Author: Agah Karakuzu <[email protected]> Date: Wed Nov 11 14:32:20 2020 -0500 Wording commit 66ba5a5 Author: Agah Karakuzu <[email protected]> Date: Wed Nov 11 14:31:40 2020 -0500 RECOMMENDED --> MUST for adding an application def to the appdx commit 5611fac Author: Agah Karakuzu <[email protected]> Date: Wed Nov 11 14:29:21 2020 -0500 Improve the appendix commit 6d3bbea Author: Agah Karakuzu <[email protected]> Date: Mon Nov 2 16:12:04 2020 -0500 Address suggestions by @tsalo commit bcb0223 Author: Agah Karakuzu <[email protected]> Date: Tue Oct 27 21:37:26 2020 -0400 [ADD] Link to the appendix commit 44e760f Author: Agah Karakuzu <[email protected]> Date: Tue Oct 27 21:28:38 2020 -0400 [ADD] Appendix - File collections - For parametrically linked file collections commit 523b1c1 Author: Agah Karakuzu <[email protected]> Date: Tue Oct 27 21:27:52 2020 -0400 [ADD] Parametrically linked file collections - Description * ENH: Link to all BEP-001 entities * STY: Escape asterisk, adjust table widths * Add suggestions by @sappelhoff * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/99-appendices/10-file-collections.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Thank you @effigies! Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Agah Karakuzu <[email protected]> * [ENH] BEP001 - RepetitionTimeExcitation and RepetitionTimePreparation (#671) * [ENH] Improve TR definitions - Include RepetitionTimePreparation - Include RepetitionTimeExcitation - Add explanation on the former RepetitionTime field. * Typo * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * ENH: Allow for variable RTE/P * STY: Update table widths * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md * Change ref to DOI @Remi-Gau * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> * Update config.yml * Update readthedocs.yml Co-authored-by: bids-maintenance <[email protected]> Co-authored-by: Agah <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]>
* [ENH] Bep 005: Arterial Spin Labeling (#652) Co-authored-by: Patricia Clement <[email protected]> Co-authored-by: Stefan Appelhoff <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Julia Guiomar Niso Galán <[email protected]> Co-authored-by: Remi Gau <[email protected]> * Update table errors * correction for table issues 'common metadata for PCASL/PASL' * correction latin phrases * Corrections tables * correction tables Cases * added link pepolar * deleted 'don't discriminate between types of labeling based on comment Thijs Van Osch * Moved MagneticFieldStrenght asl requirement to common * removed scaling factor info based on comment gllmflndn Removed: all ancillary scaling factors should be taken into account in the conversion to BIDS, which is why BIDS does not provide a separate scaling factor field other than the NIfTI header. * added MRAcquisitionType added in the common - sequence specifics table Reason: for ASL, defining 2D and 3D is required. This used to be added in the PulseSequenceType field, but this should stay a free text field. Therefore, the MRAcquisitionType field is added (based on DicomTag). * removed PulseSequenceType from the ASL part The 2D/3D information needed for ASL, is now moved to the new MRAcquisitionType field. * added required common fields sentence Some Common fields are required for ASL. To stress this out, we added a sentence for the *_as.json and the *_m0scan.json. * adaptation requirements for m0scan.json * adapted requirement level common fields for m0scan * update sentence required common fields for asl.json * Update common field EchoTime * Removed EchoTime from ASL-table * update FlipAngle in common RF and Contrasts * remove FlipAngle from ASL fields * update Dependency table ASL for MRAcquisitionType * update SliceTiming in common fields * remove SliceTiming from ASL tables * added (FA) for FlipAngle * update common RF and Contrast for table check * LabelingPulseDuration and LabelingPulseInterval in ms instead of s * required common metadatafields update RepetitionTime or VolumeTiming to RepetitionTimePreparation * update flipangle for timeseries versus file collections * update echotime for timeseries * update timing table (fence) * update definition BackgroundSuppressionPulseTime * removed common metadata fields from asl table MagneticFieldStrenght, EchoTime, SliceTiming, VolumeTiming, RepetitionTime, FlipAngle, PulseSequenceType * update scaling of asl and m0 files - update scaling description for nifti files - created separate scaling section (to highlight this important quantification factor), and M0 section - moved scaling section and M0 section after aslcontext section * update broken link in EchoTime (Timing Parameters) * update broken link (2) EchoTime * Update broken links FlipAngle * LabelingType changed to ArterialSpinLabelingType - changed in fields - Changed in dependency table - Dependency table: additionally removed repetitiontime/volumetiming parts * Update dependency table based on new M0Type and M0Estimate fields * Update new M0 field strategy M0 field becomes M0Type, with addition field M0Estimate. Also corrected IntendedFor description in m0 specific fields * TEMPORARY: disable 'strict' docs build this allows the build to proceed despite broken links * changed fail_on_warning to true Intermediate solution to get BEP005 build untill the missing link is available * see previous commit * update bep005 with latest master + resolve conflict (#689) * [DOC] Auto-generate changelog entry for PR #677 * [ENH] BEP001 - New entities: inv & mt (#681) * add entity inv * add mt: * add MT specific metadata * Generate entity.md * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Stefan Appelhoff <[email protected]> * Update src/schema/entities.yaml Co-authored-by: Stefan Appelhoff <[email protected]> * table fix by @sappelhoff * Conditional requirement Co-authored-by: Stefan Appelhoff <[email protected]> * [ENH] BEP001 - Entity-linked file collections (#688) * Squashed commit of the following: commit 8e1ac5f Author: Stefan Appelhoff <[email protected]> Date: Fri Nov 27 11:09:24 2020 +0100 fix links and latin commit 27c9b41 Author: Stefan Appelhoff <[email protected]> Date: Fri Nov 27 10:03:51 2020 +0100 remove latin Co-authored-by: Chris Markiewicz <[email protected]> commit 7d87070 Author: Agah <[email protected]> Date: Mon Nov 23 08:46:47 2020 -0500 Update src/99-appendices/10-file-collections.md Co-authored-by: Chris Markiewicz <[email protected]> commit b137a9e Author: Agah <[email protected]> Date: Mon Nov 23 08:45:19 2020 -0500 Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> commit d7897b8 Author: Agah <[email protected]> Date: Mon Nov 23 08:44:40 2020 -0500 Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> commit 09e9d32 Author: Agah <[email protected]> Date: Mon Nov 23 08:44:15 2020 -0500 Commit suggestion Co-authored-by: Chris Markiewicz <[email protected]> commit 009a9ab Author: Agah Karakuzu <[email protected]> Date: Mon Nov 23 08:43:00 2020 -0500 Add file-collecitons appendix to the TOC commit 8f6c4df Author: Agah Karakuzu <[email protected]> Date: Mon Nov 23 08:41:13 2020 -0500 Address suggestion by @tsalo commit 45210a9 Author: Agah Karakuzu <[email protected]> Date: Wed Nov 11 14:32:20 2020 -0500 Wording commit 66ba5a5 Author: Agah Karakuzu <[email protected]> Date: Wed Nov 11 14:31:40 2020 -0500 RECOMMENDED --> MUST for adding an application def to the appdx commit 5611fac Author: Agah Karakuzu <[email protected]> Date: Wed Nov 11 14:29:21 2020 -0500 Improve the appendix commit 6d3bbea Author: Agah Karakuzu <[email protected]> Date: Mon Nov 2 16:12:04 2020 -0500 Address suggestions by @tsalo commit bcb0223 Author: Agah Karakuzu <[email protected]> Date: Tue Oct 27 21:37:26 2020 -0400 [ADD] Link to the appendix commit 44e760f Author: Agah Karakuzu <[email protected]> Date: Tue Oct 27 21:28:38 2020 -0400 [ADD] Appendix - File collections - For parametrically linked file collections commit 523b1c1 Author: Agah Karakuzu <[email protected]> Date: Tue Oct 27 21:27:52 2020 -0400 [ADD] Parametrically linked file collections - Description * ENH: Link to all BEP-001 entities * STY: Escape asterisk, adjust table widths * Add suggestions by @sappelhoff * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/99-appendices/10-file-collections.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Thank you @effigies! Co-authored-by: Chris Markiewicz <[email protected]> * Update src/02-common-principles.md Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Agah Karakuzu <[email protected]> * [ENH] BEP001 - RepetitionTimeExcitation and RepetitionTimePreparation (#671) * [ENH] Improve TR definitions - Include RepetitionTimePreparation - Include RepetitionTimeExcitation - Add explanation on the former RepetitionTime field. * Typo * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * ENH: Allow for variable RTE/P * STY: Update table widths * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md * Change ref to DOI @Remi-Gau * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> * Update config.yml * Update readthedocs.yml Co-authored-by: bids-maintenance <[email protected]> Co-authored-by: Agah <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> * update required common fields for m0scan.json added EchoTime and FlipAngle in case LookLocker is true * update dependency table if PCASL : LabelingDuration is required if PCASL: LabelingDuration should not be filled in * update depedency table removed 'PASL' LAbelingDuration should not be filled in * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * update description aslcontext Co-authored-by: Chris Markiewicz <[email protected]> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * update PCASLType and CASLType Co-authored-by: Chris Markiewicz <[email protected]> * updated required into REQUIRED for asl.json and m0scan.json (common fields * Added SummaryImages Added images: asl_pasl_boluscutoff_false.png asl_pasl_boluscutoff_true_q2tips.png asl_pasl_boluscutoff_true_quipssII.png asl_pcasl_labeling_pulses.png asl_pcasl_sequence.png * create Appendix 11 for ASL * Added asl_flowchart.png * added subtitles for linking from main specification * Added appendix link for control and label * added appendix for asl sequences in general * added appendix for (P)CASL * added appendix for PASL * added appendix for dependency table (flowcharts) * MNT: Add ASL appendix to TOC * TYPO: correct ASL appendix entry in tocwq * fix md style * Update src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Co-authored-by: Chris Markiewicz <[email protected]> * update table RF and Contrast * update link Appendix XI to Appendix XI - ASL * corrected link to appendix for PASL * Update aslcontext cases Case 1: changed asl.json to asl.nii[.gz`] Removed: The optional deltam or cbf volumes should be stored and specified as derivative. Case 2: Removed: The optional cbf volumes should be stored and specified as derivative. * Added aslcontext.tsv cases this is moved from main spec bep005 to appendix XI - ASL * Update aslcontext cases - moved three cases to appendix XI - ASL - added link to appendix XI - added: Note that the raw images, including the m0scan, may also be used for quality control. * corrected subtitles cases * improve linebreaks in ASL appendix * fix VascularCrushingVENC link Based on comment of Stefan Appelhof, the Data Type for VascularChrushingVENC was fixed: [number][] or \[array][] of [numbers][] to [number][] or [array][] of [numbers][] * added single column requirement for aslcontext.tsv based on comment of Stefan Appelhof, the need for a single column was added in the introductory text for aslcontext.tsv, before the 3 cases. * Changed VascularCrushingVENC to VascularCrushingVenc * [SCHEMA] Add ASL to schema (#703) * Add ASL to schema files and regenerate entity table. * Fix style issue. * Add fmap-format m0scan to schema. * update M0Estimate fields Added 'Referring to the M0 of blood' * update LabelingDuration Added additional information for LabelingDuration * update labeling.jpg into *_asllabeling.jpg * Change labeling to asllabeling. (#709) * RF: Renumber ASL appendix to XII Co-authored-by: Henk Mutsaerts <[email protected]> Co-authored-by: Patricia Clement <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Julia Guiomar Niso Galán <[email protected]> Co-authored-by: Remi Gau <[email protected]> Co-authored-by: bids-maintenance <[email protected]> Co-authored-by: Agah <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Taylor Salo <[email protected]>
@agahkarakuzu @sappelhoff @KirstieJane @tsalo I have been trying to add support for some of these changes to dcm2niix. I have a few issues
|
Hi @neurolabusc, thank you so much for looking into this! Happy to see that
For
Again, these are for Siemens. |
|
|
@agahkarakuzu Thanks. Can you please test dcm2niix development version v1.0.20210616 and validate the performance:
|
I'll do my best to cover BEP001 methods. @ChristophePhillips @lazaral do you have some DICOM images to share for the @neurolabusc is it possible to define simple conditions and calculations within I will test out the version you mentioned as soon as possible. |
I tested
The field
|
|
Thank you @neurolabusc!
But it is possible to define a parameter based on multiple fields and conditions (without manual intervention), right? |
Yeah I should be able to pull out one DICOM file per 3D image in one of our MPM acquisition. It might be a good idea though to try and get the same from Martina Callaghan's open data set (I can ask her). This way we would have the open data in DICOM and NIfTI format. Thought? |
Hi @ChristophePhillips @agahkarakuzu, asking Martina for the DICOMs of the open MPM dataset is definitely a great idea. Also, if you're mostly interested in the DICOM metadata for MPMs, you can actually find a lot of them in the NIFTI's json file, since the standard hMRI pipeline copies and pastes a lot of DICOM metadata directly into the nifti json file, if I remember correctly. The link to the openly-available dataset (with the jsons in question) is here: https://owncloud.gwdg.de/index.php/s/iv2TOQwGy4FGDDZ |
@lazaral the openly available dataset you link to is insufficient to help us:
@agahkarakuzu the goal of dcm2niix is to populate all BIDS fields where we can determine them reliably and accurately from the source data. If you can tell me the formula to derive these new tags from the DICOM tags or CSA header, it would be great to include them. |
Headnote
A while ago we started #508, where you can find information on the overall purpose of
BEP001
. The PR #508 was closed as discussions called for some major revisions and to splitBEP001
pull requests into manageable parts.Dear BIDS community,
In #668, we proposed the common principle of
entity-linked file collections
, which requires the addition of several new entities (inv
,flip
,mt
) for the common use cases of quantitative MRI applications. Recently,flip
has been merged #672 🎊 .In this Part-4 of BEP001 pull requests, we would like to propose including the two remaining entities:
inv
InversionTime
mt
MTState
** Several metadata fields have been added under the
Sequence Specifics
section for the MT-based application, including this key.With
echo
,flip
andpart
already present, this last addition opens the way for introducing 8 commonly used qMRI methods (e.g.MP2RAGE
andMPM
), which in turn enables the calculation of 13+ parametric maps! As the BEP001 team, we did our best to focus on introducing qMRI applications that we believe find the most common user base.After merging this PR, #671 and #668, we will make pull requests to introduce suffixes for these methods and quantitative maps along with a qMRI appendix.
On behalf of the BEP001 core team:
Gilles de Hollander (@Gilles86), Alberto Lazari (@lazaral), Christophe Phillips (@ChristophePhillips), Kirstie Whitaker (@KirstieJane), Tibor Auer (@tiborauer).
Best regards.