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.

Sign up for GitHub

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

Jump to bottom

antsSliceRegularizedRegistration: Larger OSX-Linux difference in recent build #976

Closed
jcohenadad opened this issue Mar 22, 2020 · 11 comments
Closed

antsSliceRegularizedRegistration: Larger OSX-Linux difference in recent build #976

jcohenadad opened this issue Mar 22, 2020 · 11 comments

Comments

@jcohenadad
Copy link

Hi, while investigating an issue related to slow processing time on some systems, I recently re-built ANTs binaries on Ubuntu 16.04 and OSX. Good news is: processing time is much faster with the recent built on Ubuntu 16.04:

ANTs Version: 2.3.1.dev145-g7ed2b

time antsSliceRegularizedRegistration -t 'Translation[0.5]' -m 'MeanSquares[dest_RPI_crop.nii,src_reg_crop.nii,1,4,Regular,0.2]' -p 5 -i 50 -f 1 -s 0 -v 1 -o '[step1,src_reg_crop_regStep1-new.nii]'
Loop0 polyerr: 0.0995983 image-metric 0.225547
Loop1 polyerr: 0.00827533 image-metric 0.225138

real	0m4.414s
user	0m24.775s
sys	0m12.730s

ANTs Version: 2.1.0.post359-g3df42

time antsSliceRegularizedRegistration -t 'Translation[0.5]' -m 'MeanSquares[dest_RPI_crop.nii,src_reg_crop.nii,1,4,Regular,0.2]' -p 5 -i 50 -f 1 -s 0 -v 1 -o '[step1,src_reg_crop_regStep1.nii]'
Loop0 polyerr: 0.139908 image-metric 0.226171
 polyx -0.496811 1.50435 -0.990867 -0.177755 0.221031 iceptx 0.0654782
 polyy -1.83812 12.1132 -25.9729 23.1219 -7.39511 icepty -0.0206768
Loop1 polyerr: 0 image-metric 0.226171
 polyx -0.496811 1.50435 -0.990867 -0.177755 0.221031 iceptx 0.0654782
 polyy -1.83812 12.1132 -25.9729 23.1219 -7.39511 icepty -0.0206768

real	1m5.092s
user	0m43.118s
sys	2m8.049s

However, there is a much larger difference in results between OSX and Linux for the more recent version. Version 2.1.0.post359-g3df42 shows a difference at the 10^-14 decimal while version 2.3.1.dev145-g7ed2b shows a difference at the 10^-2 decimal. See example for the first three lines of the Tx params:

ANTs Version: 2.3.1.dev145-g7ed2b

OSX Linux
0.0510465709765176 0.064159965052827
-0.00246455359150863 0.000141873553017993
-0.0411416921788 -0.0435655048206819

ANTs Version: 2.1.0.post359-g3df42

OSX Linux
0.0498505333483797 0.0498505333483799
-0.00060664358328652 -0.000606643583286381
-0.0348264242930449 -0.034826424293045

Data + Full Results here:
antsSliceRegularizedRegistration.zip

I was wondering if this is a known behaviour and if you have some clues about what could have introduced such larger discrepancies in the more recent build.

Thanks for your insights!
@cookpa
Copy link
Member

cookpa commented Mar 22, 2020

What's the test-retest difference on each platform? It's possible that successive runs on the same platform are also more variable than they used to be.

@jcohenadad
Copy link
Author

ah! Good catch. Indeed, successive testing yields different results. See below first row for Tx:

0.0699915216915313
0.0660665386223871
0.0425164161621468
0.0442404901924251
0.0641184696378914

Which is not expected given that the samplingStrategy was set Regular instead of Random. Full syntax:

time antsSliceRegularizedRegistration -t 'Translation[0.5]' -m 'MeanSquares[dest_RPI_crop.nii,src_reg_crop.nii,1,4,Regular,0.2]' -p 5 -i 50 -f 1 -s 0 -v 1 -o '[step1,src_reg_crop_regStep1.nii]'

@gdevenyi
Copy link
Contributor

Regular includes a random perturbation on the grid sampling

@cookpa
Copy link
Member

cookpa commented Mar 23, 2020

Regular includes a random perturbation on the grid sampling

Correct, but this has been the case for some time.

More discussion here

https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues

@jcohenadad
Copy link
Author

additional info: when setting samplingPercentage to 1 (instead of 0.2 previously), I also get a non-null variance on the results.

@gdevenyi
Copy link
Contributor

I think the proper way to specify dense sampling is "None", rather than Regular,1.0

@stnava
Copy link
Member

stnava commented Mar 23, 2020

also, for reproducibility, one should set the ants seed and nthreads to 1.

@cookpa
Copy link
Member

cookpa commented Mar 23, 2020

also, for reproducibility, one should set the ants seed and nthreads to 1.

Thanks @stnava - I'm not sure if this program supports a fixed random seed, but if not it probably should.

Single threading will also remove another source of precision loss, though in my experiments linked above, it was a much smaller delta than the random sampling variation.

@jcohenadad
Copy link
Author

also, for reproducibility, one should set the ants seed and nthreads to 1.

Yes, these env variables were already set properly.

I think the proper way to specify dense sampling is "None", rather than Regular,1.0

That worked! Thanks a lot. I would suggest to add this option in the usage:
samplingStrategy={Regular,Random} --> samplingStrategy={Regular,Random,None}

@jcohenadad jcohenadad mentioned this issue Mar 23, 2020
Closed
@cookpa
Copy link
Member

cookpa commented Mar 25, 2020

I updated the usage (ea5a7f1).

Wiki page for metric sampling strategies

https://github.com/ANTsX/ANTs/wiki/antsRegistration-metric-sampling-strategies

@cookpa
Copy link
Member

cookpa commented Mar 27, 2020

Closing for now, feel free to re-open if there's additional questions

@cookpa cookpa closed this as completed Mar 27, 2020
@kousu kousu mentioned this issue Apr 21, 2020
Merged
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue Apr 25, 2020
@kousu
Use fully dense sampling when motion correcting.
1b1d1be 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue Apr 25, 2020
@kousu
Use fully dense sampling when motion correcting.
02f72a9 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue Apr 25, 2020
@kousu
Use fully dense sampling when motion correcting.
065e177 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue May 4, 2020
@kousu
Use fully dense sampling when motion correcting.
cbaa4bc 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue May 9, 2020
@kousu
Use fully dense sampling when motion correcting.
63e0078 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue May 11, 2020
@kousu
Use fully dense sampling when motion correcting.
530af4f 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue May 17, 2020
@kousu
Use fully dense sampling when motion correcting.
d77b77a 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
kousu added a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue May 17, 2020
@kousu
Use fully dense sampling by default when motion correcting.
2450f8b 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

I think ideally moco.py would thinly wrap antsRegistration, and
expose both .sampling_strategy and .sampling_percentage directly,
but until now we've always hardcoded 'samplingStrategy=Regular'
so in the interest of not rocking the boat too much all I did
was add a 'sampling=None' case, *and set it as the default*

Updated version re feedback: #2642 (comment)
jcohenadad pushed a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue May 22, 2020
@kousu @jcohenadad
Use fully dense sampling when motion correcting.
f19d11d 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

Because we are hard-coding the sampling rate I removed it from ParamMoco
entirely. If we want to support it properly we would have to pass
.sampling_strategy and .sampling_percentage separately.
jcohenadad pushed a commit to spinalcordtoolbox/spinalcordtoolbox that referenced this issue May 22, 2020
@kousu @jcohenadad
Use fully dense sampling by default when motion correcting.
fb64a61 
Upon upgrading our ANTS build we found that motion correction was no
longer deterministic (#2642 (comment)).

The ANTS maintainers suggested setting samplingStrategy=None
(ANTsX/ANTs#976 (comment))
to get deterministic output, but they also warned that this will induce
a variance-for-bias tradeoff:

* https://github.com/ANTsX/ANTs/wiki/antsRegistration-reproducibility-issues
* ANTsX/ANTsR#210 (comment)
* Thévenaz, P., Bierlaire, M., & Unser, M. (2008).
  Halt on sampling for image registration based on mutual information.
  Sampling Theory in Signal and Image Processing, 7(2).
  <http://bigwww.epfl.ch/preprints/thevenaz0602p.pdf>

I think ideally moco.py would thinly wrap antsRegistration, and
expose both .sampling_strategy and .sampling_percentage directly,
but until now we've always hardcoded 'samplingStrategy=Regular'
so in the interest of not rocking the boat too much all I did
was add a 'sampling=None' case, *and set it as the default*

Updated version re feedback: #2642 (comment)
@kousu kousu mentioned this issue Jul 31, 2020
Closed
This was referenced Nov 16, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stnava @cookpa @jcohenadad @gdevenyi