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

population_template: Fix linear drift correction #2802

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

Lestropie
Copy link
Member

Found this one while porting code for #2678.

Problem appears to have been introduced here in 5ad072f as part of #2471.

It does not affect master, as the drift correction functionality does not yet exist there.

I'm posting as a draft for now, as currently the average transformation is recomputed at every linear registration iteration, whereas it should only need to be done once; but I'll want to test a little more before committing that.

@maxpietsch: Do you have any good exemplar data with a clear drift?

Transforms purportedly averaged across the cohort were erroneously equivalent to the transform of the last input.
Copy link

clang-tidy review says "All clean, LGTM! 👍"

@Lestropie
Copy link
Member Author

@daljit46: There's a MacOSX CI failure on this one related to cmake that's unrelated to the changeset.

@daljit46
Copy link
Member

This looks like a symlinks issue, but unfortunately I still haven't gotten my Macbook yet so I'm unable to test it. Perhaps @bjeurissen could take a look and see what's going on?

- Only compute the reference transform for linear drift correction once, rather than overwriting it every linear iteration.
- If performing linear drift correction, but no initial alignment was performed, then use the mean of the first linear iteration as the drift correction reference.
Copy link

github-actions bot commented Mar 3, 2024

clang-tidy review says "All clean, LGTM! 👍"

@Lestropie Lestropie marked this pull request as ready for review March 3, 2024 23:08
@Lestropie
Copy link
Member Author

I have ensured that population_template at least executes for both combinations of -linear_no_drift_correction (present vs. absent) and -initial_alignment none vs. default (mass). Script tests fail due to divergence with previously generated data, but that is due either to the drift correction itself, or something else upstream. I think that I will defer testing to #2811 once 3.1.0 is closer; for each failed test I can do git bisects to figure out what introduces changes, and regenerate reference data if required.

@Lestropie Lestropie merged commit c319696 into dev Mar 4, 2024
6 checks passed
@Lestropie Lestropie deleted the population_template_linear_drift_fix branch March 4, 2024 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants