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

Switch to using mean magnitude for output moco_params.tsv file used for QC #4396

Merged
merged 8 commits into from Mar 14, 2024

Conversation

mguaypaq
Copy link
Member

@mguaypaq mguaypaq commented Mar 7, 2024

The purpose of this QC file is to identify the amount of motion in each volume (that is, at each time point), so that motion-related fluctuation in the signal can be corrected for.

Having the average of the signed X and Y values is not useful for this, because a lot of left-right motion (or front-back motion) could mostly cancel out, and look the same as not having much motion at all. This is potentially confusing, and prone to misuse.

In contrast, the mean magnitude is only small when there is not much total motion, and it is scale-invariant with the number of Z slices.

Note: the formula for magnitude, $\sqrt{X^2+Y^2}$, only makes sense when $X$ and $Y$ have the same units (that is, millimeters, not voxels). So, I considered whether we should also use the voxel dimensions in the calculation, and concluded that we shouldn't:

  • I ran the sct_fmri_moco command from our tutorial to get a baseline file moco_params.tsv.
  • I rescaled the input image and mask from (1mm, 1mm, 3mm) to (2mm, 2mm, 6mm) and ran the command again. This was the same number of voxels, just different metadata. All the numbers in the resulting file moco_params.tsv approximately doubled. Which means that the coordinates are in physical units, not integer array coordinates.
  • I also rescaled a single axis, from (1mm, 1mm, 3mm) to (2mm, 1mm, 3mm) and ran the command again. This time, the numbers in moco_params.tsv were very different, basically unrelated. Which further confirms that ANTS registration takes into account physical units, I guess.

Fixes #4344.

The purpose of this QC file is to identify the amount of motion in each volume
(that is, at each time point), so that motion-related fluctuation in the signal
can be corrected for.

Having the average of the signed X and Y values is not useful for this, because
a lot of left-right motion (or front-back motion) could mostly cancel out, and
look the same as not having much motion at all. This is potentially confusing,
and prone to misuse.

In contrast, the mean magnitude is only small when there is not much total
motion, and it is scale-invariant with the number of Z slices.
@mguaypaq mguaypaq added enhancement category: improves performance/results of an existing feature SCT API: moco.py context: labels Mar 7, 2024
@mguaypaq mguaypaq added this to the 6.3 milestone Mar 7, 2024
@mguaypaq mguaypaq self-assigned this Mar 7, 2024
@mguaypaq mguaypaq added the sct_fmri_moco context: label Mar 7, 2024
@mguaypaq
Copy link
Member Author

mguaypaq commented Mar 7, 2024

I fixed the description of the output files in the help message for sct_fmri_moco to match the new output. The only other script that uses moco.py is sct_dmri_moco, but that one doesn't talk about the TSV output.

@mguaypaq mguaypaq marked this pull request as ready for review March 7, 2024 19:50
@mguaypaq
Copy link
Member Author

mguaypaq commented Mar 7, 2024

Ah, I forgot to update the tests. Sorry!

I just used the current outputs, since we don't really have
any principled values to compare with.
@mguaypaq
Copy link
Member Author

mguaypaq commented Mar 7, 2024

Bug fixed, tests updated, ready for review again.

Copy link
Member

@joshuacwnewton joshuacwnewton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have two miniscule suggestions -- the bulk of my comments are just double-checking my own understanding. But, this mostly LGTM!

spinalcordtoolbox/moco.py Show resolved Hide resolved
testing/cli/test_cli_sct_dmri_moco.py Show resolved Hide resolved
spinalcordtoolbox/moco.py Outdated Show resolved Hide resolved
testing/cli/test_cli_sct_dmri_moco.py Show resolved Hide resolved
spinalcordtoolbox/scripts/sct_fmri_moco.py Show resolved Hide resolved
@joshuacwnewton joshuacwnewton changed the title Switch to using mean magnitude for moco QC Switch to using mean magnitude for output moco_params.tsv file used for QC Mar 8, 2024
@joshuacwnewton
Copy link
Member

((Updated the PR title, just because there also exists a "QC Report" for motion correction, and I was a little confused at first, hehe.))

mguaypaq added a commit that referenced this pull request Mar 13, 2024
The columns were changed in #4396, but the tutorial changes
should only be merged when SCT v6.3 gets released, since the
website updates immediately.
@mguaypaq mguaypaq merged commit d72ea04 into master Mar 14, 2024
20 checks passed
@mguaypaq mguaypaq deleted the mgp/mean-magnitude branch March 14, 2024 18:57
joshuacwnewton pushed a commit that referenced this pull request Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement category: improves performance/results of an existing feature SCT API: moco.py context: sct_fmri_moco context:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Averaging of motion parameters should use unsigned displacement
2 participants