Add -normalize-PAM50 argument to metric extraction script to allow interpolation to PAM50 space - #5236
Open
valosekj wants to merge 25 commits into
Open
Add -normalize-PAM50 argument to metric extraction script to allow interpolation to PAM50 space#5236valosekj wants to merge 25 commits into
-normalize-PAM50 argument to metric extraction script to allow interpolation to PAM50 space#5236valosekj wants to merge 25 commits into
Conversation
…_extract_metric` to interpolate the extracted metric values into PAM50 dimensions per slice
valosekj
commented
Jun 3, 2026
valosekj
marked this pull request as ready for review
June 8, 2026 06:42
Member
Author
|
@sandrinebedard reviewed the PR. @joshuacwnewton, could you also review the PR? Thank you 🙏 |
Member
|
In the progress of reviewing this PR. Just trying to figure out why the tests are failing! 🤔 |
joshuacwnewton
requested changes
Jun 11, 2026
joshuacwnewton
left a comment
joshuacwnewton
left a comment
Member
There was a problem hiding this comment.
There are a lot of good things to this PR (thanks for adding tests!) but I think I may have found an important thing to discuss in terms of the order of aggregation and interpolation.
joshuacwnewton
changed the title
sct_extract_metric: normalize DWI to PAM50-normalize-PAM50 argument to metric extraction script (to allow interpolating DTI in PAM50 space)
joshuacwnewton
added
sct_extract_metric
…ic_normalize_pam50
valosekj
changed the title
-normalize-PAM50 argument to metric extraction script (to allow interpolating DTI in PAM50 space)-normalize-PAM50 argument to metric extraction script to allow interpolation to PAM50 space
Since `-normalize-PAM50` already requires `-perslice 1`, the two separate `extract_metric()` calls were redundant. Context: #5236 (comment)
interpolate_metrics() expects compute_shape()'s output form (one Metric object per metric, multiple slices), while _build_pam50_agg_metric() works with extract_metric()'s output form (one dict per slice, multiple metrics). Context: #5236 (comment)
sct_extract_metric aggregates in native space then interpolates to PAM50, while sct_process_segmentation interpolates to PAM50 first then aggregates. Context: #5236 (comment)
joshuacwnewton
self-requested a review
August 12, 2026 12:40
joshuacwnewton
approved these changes
Aug 12, 2026
joshuacwnewton
left a comment
joshuacwnewton
left a comment
Member
There was a problem hiding this comment.
I think this LGTM! I just had a few small comments to address the remaining open conversations, but I think the overall structure/plan of this PR seems sound, and I appreciate the new tests. :)
Co-authored-by: Joshua Newton <joshuacwnewton@gmail.com>
Thanks Joshua! Context: #5236 (comment)
Member
Author
Thank you for your review and suggestions, @joshuacwnewton! Let me test the PR on the spine-generic dataset before merging it! |
Member
Sounds good to me! :) |
valosekj
added a commit
to spinalcordtoolbox/PAM50-normalized-metrics
that referenced
this pull request
Aug 22, 2026
…oolbox#5236 - comment out perlevel native extraction-- used in another analysis; not needed now - update config_process_data_dwi.json
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Checklist
Description
This PR implements per-slice interpolation to
sct_extract_metricto enable computing DTI maps in the native and then interpolating them per vert level to the PAM50 template space.I'm reusing spinalcordtoolbox.metrics_to_PAM50.interpolate_metrics.
Example usage
amu01_dwi_moco_FA.csv sample:
Files:
PR5236.zip
TODO
Due to high DWI slice thickness (5mm), there are “steps” at the “interface” of discs (see slide 8) --> try to include one slice above and below each level for the interpolation.
Linked issues
Related: spinalcordtoolbox/PAM50-normalized-metrics#41