MRtrix3 / mrtrix3

MRtrix3 provides a set of tools to perform various advanced diffusion MRI analyses, including constrained spherical deconvolution (CSD), probabilistic tractography, track-density imaging, and apparent fibre density
http://www.mrtrix.org
Mozilla Public License 2.0
295 stars 182 forks source link

population_template: Fix linear drift correction #2802

Closed Lestropie closed 9 months ago

Lestropie commented 9 months ago

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?

github-actions[bot] commented 9 months ago

clang-tidy review says "All clean, LGTM! :+1:"

Lestropie commented 9 months ago

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

daljit46 commented 9 months ago

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?

github-actions[bot] commented 9 months ago

clang-tidy review says "All clean, LGTM! :+1:"

Lestropie commented 9 months ago

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.