[ENH]: BEP046, diffusion tractography#2333
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2333 +/- ##
=======================================
Coverage 83.07% 83.07%
=======================================
Files 22 22
Lines 1696 1696
=======================================
Hits 1409 1409
Misses 287 287 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @arokem and @francopestilli, I hope you are well. We are getting ready to release data from our NIH BRAIN CONNECTS UM1 and are reviewing BEP046. Prior to reviewing BEP046, our plan was to release the tractography data as TRK files since that is the format we use throughout most of our processing pipelines. I see that the plan for BEP046 is to only support TRX files. We would like you to reconsider also supporting TRK files. From our perspective, TRK and TCK represent the most widely used tractography formats. We believe BEP046 would benefit from supporting these established formats, similar to how the BIDS microscopy spec supports multiple formats (PNG, TIFF, OME-TIFF, and OME-Zarr), even through OME-Zarr appears to be the most cutting-edge option. Presumably the choice of the NIfTI format for neuroimaging data in the BIDS spec was much easier as that had been the most commonly used format for many years. Perhaps once the microscopy and tractography worlds settle on their respective formats the BIDS spec could support a single format, but in the meantime it would be beneficial to support the most commonly used formats. Additionally, I am unclear as to why the need to use TRK version 2 as outlined in the Google Doc is a 'non-starter' and would appreciate clarification on this point. Thanks for considering and I would be happy to join your working group meetings to discuss further. |
|
Hi @kabilar: thanks for contributing to this discussion! I would also suggest to read this discussion that led to the development of TRX: nipy/nibabel#942 and the document that laid out the initial plans for its specification: https://docs.google.com/document/d/1GOOlG42rB7dlJizu2RfaF5XNj_pIaVl_6rtBSUhsgbE/edit?tab=t.0#heading=h.c6igqydj1hrf. For obvious reasons, BIDS has traditionally been quite resistant to proliferation of file formats, where that can be avoided. I believe that it can be avoided here, while TRX offers a good set of features for the community to work with and growing software support (now in Python, C++, Javascript, and even Pascal!). In the meanwhile, I will separately send you information about the meetings of the IST Standardization Unit, so you can join the conversation there. |
|
Surfice already supports TRX (though it seems to be a while since a release was made.) DSI Studio and MRTRIX will both be supporting trx in the near future (e.g., see PR and PR). MI-Brain eventually too, according to my understanding, but I don't exactly know what the status of support for TRX is there. Niivue opens up the possibility to relatively easily design a variety of tools for specific uses. I think that brainlife uses niivue for some of their tools, so it's possible that they already have some tools for editing tractograms in TRX (@francopestilli, can you comment?) I realize that I forgot to cc @bids-standard/bep046 when posting this PR originally. I hope that others in that team can chime in as well. |
|
What are the tools that users you are referring to currently using with trk and tck files? I forgot to mention that @mattcieslak has already implemented TRX reading for ITK (PR), so I believe that should cover some more tools (e.g., 3D slicer, I think? Maybe also Freeview? There's also a browser-based freeview that should already read/write TRX thanks to niivue) |
|
You'd be able to do this in DSI Studio after frankyeh/DSI-Studio#114 is approved |
|
@ayendiki Is this repository helpful? https://github.com/tee-ar-ex/trx-python |
|
some other news, TRX support is now available in ITK as a remote module! InsightSoftwareConsortium/ITK#5925 (comment) |
jhlegarreta
added
BEP
tractography
| Where `tract` is the anatomical/structural entity that is being imaged, and | ||
| `track` uses as its value one of the items in a controlled vocabulary. |
There was a problem hiding this comment.
Could you please provide a list of the controlled vocabulary (or a link)? Thanks.
There was a problem hiding this comment.
It's table 10 in the Google doc: https://docs.google.com/document/d/1ubDQ2RhgjnfGqoeukzEkPV9YEHhfYMERrj7-3b0c2HI/edit?tab=t.0
| track: | ||
| name: track | ||
| display_name: Tractography Method | ||
| description: | | ||
| The `track-<label>` entity identifies the tractography method | ||
| employed to reconstruct white matter fiber streamlines. | ||
| type: string | ||
| format: label | ||
| tracksys: | ||
| name: tracksys | ||
| display_name: Tracking System | ||
| description: | | ||
| The `tracksys-<label>` entity can be used as a key-value pair | ||
| to label *_motion.tsv and *_motion.json files. | ||
| It can also be used to label *_channel.tsv or *_events.tsv files | ||
| when they belong to a specific tracking system. | ||
|
|
||
| This entity corresponds to the `"TrackingSystemName"` metadata field in a *_motion.json file. | ||
| `tracksys-<label>` entity is a concise string whereas `"TrackingSystemName"` | ||
| may be longer and more human readable. | ||
| type: string | ||
| format: label |
There was a problem hiding this comment.
Since there is already an entity that uses track as part of tracksys to describe motion tracking, should we use a more distinct entity? @ayendiki had suggested trace, which I also like.
There was a problem hiding this comment.
Wouldn't that be confused with the trace of a tensor?
Would "tracking" make this less ambiguous? Or "tracto"? We could also go more abstract with "method" or "algo", but that might also ultimately conflict with other corners of the spec.
There was a problem hiding this comment.
How about "pathway", "fascicle" or "fasciculus"? I think that would resolve all concerns. Related to https://github.com/bids-standard/bids-specification/pull/2333/changes#r3464492486
There was a problem hiding this comment.
I think this demonstrates the confusion! Here, we are referring to the method used to propagate streamlines ("track") and not to the anatomical structure ("tract"). Confusing.
There was a problem hiding this comment.
🙃 Another reason to avoid "tract" when referring to the anatomical structure.
There was a problem hiding this comment.
Recommend forbidding "track" throughout. It causes far too many problems. "tract" is fine as long as there is diligence around use of "streamline" rather than "track".
francopestilli
requested review from
Lestropie,
erdalkaraca,
francopestilli and
oesteban
as code owners
|
@effigies @PeerHerholz @kimberlylray can we merge this into the spec please? |
|
@jdtournier @kabilar @erdalkaraca @oesteban can one of you please review and approve? |
|
The last series of commits (starting with b32ea9f) completes the transition of content that was previously in the google doc, so I am also going to mark the google doc as "deprecated". Disclosure: these last few commits were generated using the "Big Pickle" model on opencode, but I carefully reviewed every line and adjusted where the model had introduced things I would deem inaccurate. There are a few files (in the schema in particular) where I have tried to emulate existing patterns, but might be missing important knowledge, so I might need specifically review of these files by someone who better understands how the schema works. |
Starting to port over information from https://docs.google.com/document/d/1ubDQ2RhgjnfGqoeukzEkPV9YEHhfYMERrj7-3b0c2HI/edit?tab=t.0
Add BEP046 entities.
Use backticks for highlighting.
Co-authored-by: Kabilar Gunalan <kabilar.gunalan@gmail.com>
Co-authored-by: Kabilar Gunalan <kabilar.gunalan@gmail.com>
Removing extra line for consistency Co-authored-by: Kabilar Gunalan <kabilar.gunalan@gmail.com>
Applies this enum as the values allowed for 'track' entity. Also, enforce alphabetic order in entities.yaml.
Also, adds schema objects metadata specifications for sidecar items.
|
Now also rebased on master. |
|
|
||
| Table 2d (contents of elements of dict `StreamlineSeeding["InitialDirection"]`): | ||
|
|
||
| TODO |
There was a problem hiding this comment.
This was absent in the google doc, so is still absent here as well. Not 100% sure what this was supposed to describe originally, a random guess is that @Lestropie might have some memory of this (even though it's been a while).
There was a problem hiding this comment.
Did not finish translating the set of streamline reconstruction mechanisms to the document. But as per prior discussion, I would be stripping all of that content out of the BEP. It's not the right environment for that deconstruction, it's not the right resolution of information required for BIDS Derivatives data, some of it is too fine-grained for even tractography experts to be able to fill in, and it draws too much attention and effort away from what should actually be in the BEP.
Testing the theory that when this was identical to the value associated with the "name" key, this raised errors in testing.
This reverts commit cee4f74.
| <source-entities>[space-<space>]_[tract-<tract name>]_[track-<tracking method>]_tractogram.json | ||
| ``` | ||
|
|
||
| Where `tract` is the anatomical/structural entity that is being imaged, and |
There was a problem hiding this comment.
Alternatives proposed for "tract" are "fasc" (as in "fascicle"), "subset", "bndl" (as in "bundle"), "part", "str" (for "structure", maybe "strc" or somesuch instead?).
| sub-01_space-MNI152NLin2009cAsym_tract-wholebrain_track-eudx_tractogram.trx | ||
| sub-01_space-MNI152NLin2009cAsym_tract-wholebrain_track-eudx_tractogram.json | ||
| sub-01_space-MNI152NLin2009cAsym_tract-ArcuateFasciculus_hemi-L_track-eudx_tractogram.trx | ||
| sub-01_space-MNI152NLin2009cAsym_tract-ArcuateFasciculus_hemi-L_track-eudx_tractogram.json |
There was a problem hiding this comment.
Should values associated with "tract" be a controlled vocabulary? Having to define all possible structures that someone might want to describe would be a real bear. On the other hand, not defining this opens up the possibility that different people would name the Arcuate "ArcuateFasciculus", "ARC", "Arcuate", "FrontoTemporal", and so forth., making it hard to share informatively, integrate across datasets, etc.
There was a problem hiding this comment.
I think we should agree on a minimal set. We can ask the IST subunit to help us with the terms. I know defining them anatomically is not an easy task, but here our job is not to define them anatomically, just to provide a controlled vocabulary, and maybe provide a very broad explanation about them. I believe this is doable for a reasonable amount of tracts.
There was a problem hiding this comment.
One related idea (from @kimberlylray) as expressed just now in the IST Standarization Unit meeting is to allow for flexibility to use one of several “atlases” and then the names are defined by that atlas (for example, the HCP-1065 Young Adult Fiber Templates. Metadata would report which atlas was used.
There was a problem hiding this comment.
I wouldn't even shoot for a controlled vocabulary. Even for something as well-known as the arcuate, there is disagreement about exactly what breadth of connections that name refers to---not just in the tractography community, in the neuroanatomy community---so controlling the name doesn't standardise the data.
One could make the argument that anything relating to exactly how that tract is defined, ie. the set of criteria used to either reconstruct a tract or extract it from a whole-brain tractogram, is actually a BIDS Provenance issue, and a BEP like this shouldn't attempt to plug that gap. Because even the esoterics of how a particular software package applies that particular set of rules to the data may be consequential, so by the time you embed enough information about not only the set of rules defining the tract but also the software and the processing by which they were applied, hey presto, you've duplicated provenance. If a particular pipeline can provide some information about how the tract was extracted, e.g. a WMQL string or a filename from a piece of software containing a set of rules or a process, maybe there's no harm in it being there as long as it's not to be treated as facilitating reproduction.
There was a problem hiding this comment.
I just made a related comment below. I will delete that. I agree that we should implement this, given that not all Arcuate Fasciculi are created equal, we need to know how your arcuate was created, meaning which segmentation or atlas you used.
I was thinking to NOT allow produce different tracts by different segmentation methods, but asumen that there will be oalwaysone altas for an entire dataset, if that is a good assumption we can add the atlas / segmentation in the JSON side-car, no? @arokem @kimberlylray
| "Algorithm": ("REQUIRED", "Dict", "The contents of this dictionary are described in Table 3b."), | ||
| "AnglePerStep": ("OPTIONAL", "Float", "The maximal change in degrees in the streamline tangent between successive steps. Note that exactly how this parameter is interpreted may depend on the value of IntegrationOrder."), | ||
| "Interpolation": ("REQUIRED", "String", "Selection from the following: ['Nearest', 'Linear', 'Cubic', 'Spline']. The mechanism by which sub-voxel information is drawn from the Source based on the precise streamline location."), | ||
| "InterpolationOrder": ("REQUIRED", "Int", "The order of interpolation used (or 0 if linear)"), |
There was a problem hiding this comment.
Added here to address comments made in the google doc by @francopestilli and @jhlegarreta
There was a problem hiding this comment.
Following to the google doc comment, I am not sure whether at the time I commented on this there was a proposal for the vocabulary (i.e. linear, cubic, etc.). I am fine using that vocabulary. In that case, the order is redundant. The only concern here are splines, as they would require additional information to be reproducible. Does anybody know, out of the top of their heads, how this is handled in other BEPs where splines are used?
| - $ref: objects.enums.fact.value | ||
| - $ref: objects.enums.ifod.value | ||
| - $ref: objects.enums.sdstream.value | ||
| - $ref: objects.enums.tensor.value |
There was a problem hiding this comment.
This seems a little under-constrained. What did you do with this tensor?
|
Since this came up earlier in this thread, just a heads up that the new release of FSLeyes now supports the TRX format: https://open.oxcin.ox.ac.uk/pages/fsl/fsleyes/fsleyes/userdoc/changelog.html Hopefully that is helpful for end users of data compliant with the proposed standard, and let me know if there are other places where such support would be helpful. |
Use `anyOf` to reference `_TrackMethod` enum in the `track` schema entity. The `track` entity was using `enum: [$ref: objects.enums._TrackMethod]`, which placed the entire `_TrackMethod` object as a list element instead of producing a flat list of string values. This caused a `KeyError: 'name'` in the text renderer when iterating over enum items. Use `anyOf` instead, consistent with how group enum objects are referenced in `metadata.yaml`. The existing `flatten_enums()` in `schema.py` then correctly merges it into a flat list of allowed values.
The `make_metadata_table` macro only accepts 2-item tuples of the form
(`requirement_level, description_addendum`). The
`diffusion-derivatives.md` file was passing 3-item tuples including a
type hint as the middle element (e.g. ("OPTIONAL", "String",
"description...")), causing a
```
ValueError: too many values to unpack (expected 2)
````
error during the mkdocs build.
Remove the type string from all `make_metadata_table` tuple values in
`diffusion-derivatives.md`.
7 participants
Starting to port over information from https://docs.google.com/document/d/1ubDQ2RhgjnfGqoeukzEkPV9YEHhfYMERrj7-3b0c2HI/edit?tab=t.0
Note
We meet regularly to discuss this BEP at the IST Standardization Unit
Please reach out to @arokem (arokem@gmail.com) if you would like to join these meetings, which usually occur on the last Friday of each month at 7:30 AM PT.