SIFT2: Changes to output streamline weights units #2922
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Rather than being approximately centred around unity, streamline weights will now be written appropriately pre-multiplied such that they have units mm^2. - These units can be modified both at the command-line and in the MRtrix configuration file. - Update tcksift2 reference to OHBM Aperture article.
|
From #2138:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| ARGUMENTS | ||
| + Argument ("in_tracks", "the input track file").type_tracks_in() | ||
| + Argument ("in_fod", "input image containing the spherical harmonics of the fibre orientation distributions").type_image_in() | ||
| + Argument ("out_weights", "output text file containing the weighting factor for each streamline").type_file_out(); | ||
|
|
||
| OPTIONS | ||
| + Option ("units", "specify the physical units for the output streamline weights (see Description)") | ||
| + Argument ("choice").type_choice(SIFT2::units_choices) |
There was a problem hiding this comment.
warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
+ Argument ("choice").type_choice(SIFT2::units_choices)
^| @@ -88,7 +89,7 @@ class TckFactor : public SIFT::Model<Fixel> { | |||
|
|
|||
| void report_entropy() const; | |||
|
|
|||
| void output_factors(const std::string &) const; | |||
| void output_factors(const std::string &, const units_t) const; | |||
There was a problem hiding this comment.
warning: parameter 2 is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]
Suggested change
| void output_factors(const std::string &, const units_t) const; | |
| void output_factors(const std::string &, units_t) const; |
|
|
||
| namespace MR::DWI::Tractography::SIFT2 { | ||
|
|
||
| extern const char* const units_choices[] = { "NOS", "none", "AFD/mm", "AFD.mm-1", "AFD.mm^-1", "mm2", "mm^2", nullptr }; |
There was a problem hiding this comment.
warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]
extern const char* const units_choices[] = { "NOS", "none", "AFD/mm", "AFD.mm-1", "AFD.mm^-1", "mm2", "mm^2", nullptr };
^|
|
||
| namespace MR::DWI::Tractography::SIFT2 { | ||
|
|
||
| extern const char* const units_choices[]; |
There was a problem hiding this comment.
warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]
extern const char* const units_choices[];
^
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2138.
I've for years questioned myself about exactly how this confound should be treated. But in retrospect it's remained unaddressed for too long, and I regularly find myself having to re-explain what factors to apply when, what is automatically applied vs. what isn't, what the default should be, and what the interface should look like.
(I like having the capability to account for differences in response function magnitudes after the fact rather than require common response functions, but in retrospect it just confuses people).
I think I've landed on the right long-term solution. This does result in a difference in command output compared to
3.0.x, but the prior behaviour can also be achieved if desired. Rather than having the interface relate to "has been scaled by X / has not been scaled by X", I think the right approach is to focus on the units in which those streamline weights are provided. That is then easily reportable as metadata in the file header. The new tests hopefully prove the corresponding interpretations.