-
Notifications
You must be signed in to change notification settings - Fork 116
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
elastix 4.9 performance degradation #89
Comments
To be clear, this only happens with the MultiMetricMultiResolutionRegistration, not with the MultiResolutionRegistration |
@N-Dekker found the root cause: He is working on a fix. |
FYI, the fix that I proposed for ITK is at InsightSoftwareConsortium/ITK#351 ("PERF: Remove SystemInformation data from ResourceProbe...") |
Root cause fixed by @N-Dekker 👍 InsightSoftwareConsortium/ITK@747f3d1 Next step would be to upgrade elastix to ITK5. |
explanation is that the multi-metric created a itk::TimeProbe in every iteration, which suddenly took quite a bit of overhead (about 0.1 second in every iteration). |
This will be fixed with the new release with ITK5 support. Closing |
FYI, ITK4.13.2 also includes the |
Hi @N-Dekker and @mstaring, |
Thanks for your questions, Ricardo @rcorredorj
Correct, these elastix 4.9 binaries still have the ITK performance issue. If you would want elastix 4.9 without this performance issue, you would have to build it yourself, using a fixed version of ITK4. (>= ITK v4.13.2)
I don't think so. But is there a specific reason why you would be reluctant to use elastix 5.0? |
@N-Dekker thanks for your quick reply. Some of our applications running use ITK 4.11.0. It is going to be easier to push an upgrade from ITK 4.11.0 to 4.13.x. We actually tried to compile elastix 4.9 with ITK 4.11.0. That seems to compile properly, but I tried first to run a comparison between elastix 4.8 vs elastix 4.9 using the executables available in the releases in GitHub. However, I just noticed considerable differences on the output images using the same parameters files. Not sure now whether changing to ITK 5 or 4.13.2 will provide more consistent results with elastix 4.8 (Note: many reproducibility studies run already in our team with 4.8, this is why we were expecting to keep consistent results as much as possible) |
Actually ... I just run the same registration with the binaries in the release of elastix 5.0. I get the same results that I got with elastix 4.9, both being different compared to elastix 4.8. Then the question @N-Dekker is, were there fundamental changes in the registration algorithms and dependencies between 4.8 and later versions ? I'm not doing anything really fancy in the registration. The parameters are practically from one of the example parameters files for an Affine registration and look as follow: // C-style comments: // // The internal pixel type, used for internal computations // The dimensions of the fixed and moving image // Specify whether you want to take into account the so-called // **************** Main Components ************************** // The following components should usually be left as they are: // These may be changed to Fixed/MovingSmoothingImagePyramid. // The following components are most important: (Metric "AdvancedMattesMutualInformation") // ***************** Transformation ************************** // Scales the affine matrix elements compared to the translations, to make // Automatically guess an initial translation by aligning the // Whether transforms are combined by composition or by addition. // ******************* Similarity measure ********************* // Number of grey level bins in each resolution level, // If you use a mask, this option is important. // ******************** Multiresolution ********************** // The number of resolutions. 1 Is only enough if the expected // The downsampling/blurring factors for the image pyramids. // ******************* Optimizer **************************** // Maximum number of iterations in each resolution level: // The step size of the optimizer, in mm. By default the voxel size is used. // **************** Image sampling ********************** // Number of spatial samples used to compute the mutual // Refresh these spatial samples in every iteration, and select // ************* Interpolation and Resampling **************** // Order of B-Spline interpolation used during registration/optimisation. // Order of B-Spline interpolation used for applying the final //Default pixel value for pixels that come from outside the picture: // Choose whether to generate the deformed moving image. |
Thanks for the parameter file! Honestly I wasn't involved yet between elastix 4.8 and 4.9. But there appear some changes regarding BSpline at https://github.com/SuperElastix/elastix/releases/tag/4.9.0
HTH, Niels |
@N-Dekker Thank you so much for the fast replies. @mstaring , I notice you have done those changes mentioned by Niels. 43687e5 May these changes in the B-spline kernels affect the B-Spline interpolators? (we are just using AffineTransform; not a RecursiveBSplineTransform that internally uses the BSplineDerivativeKernelFunction2 affected by changes in kernels) |
@rcorredorj I just discussed your question with my colleagues Marius (@mstaring) and Stefan (@stefanklein). We concluded that there were no intended fundamental changes in the registration algorithms, between elastix 4.8 an 5.0. Differences between the registration results of those versions are always possible, but they should only be caused by little code improvements and bug fixes. We do think that elastix 5.0 is stable. That's certainly the intention! However, you might have a look at the log files from elastix 4.8 versus 4.9 or 5.0, to get more details about those difference. If you still suspect a fundamental change, and if you think there's something to be fixed, you may of course open a new issue at https://github.com/SuperElastix/elastix/issues/new Thanks for your interesting questions, Ricardo! |
Thank you so much @N-Dekker . I've been comparing the two versions elastix_04_8...4.9.0, but I don't have the deep knowledge of the code to know whether the core modules were affected by some bug fixes. I just checked the bug fix related to the bsplines, but didn't seem to me to touch the interpolators or anything that it's used in the simple paramaters that I'm using. It's a bit tricky because when computing the absolute difference between the result of 4.8 and 4.9 I get an image like this one (max abs. difference = 21) The results to me seem quite affected, not just by a few numerical/floating-point issues. Between 4.9 and 5.0 I get the same result, though. Considering this, you suggest anyways to use the 4.9 or newer, correct ? |
Hi, Thanks @stefanklein. The substraction between the two images gives me a resulting image with a min and max of -21 and 21 respectively: I'm just a bit surprised. I have 258949 voxels with differences < -2 (in intensity values) and 259618 voxels with differences > 2. Seems quite a lot for being numerical issues due to subvoxel operations (considering that are the same images and same parameters). The tricky part is that we are now trying to understand whether we can confirm that results with 4.9 are better than the ones with 4.8 (computing the exact metric is not that conclusive). In any case, I understand from your comments that you suggest to upgrade to more recent versions. I guess one of the bug fixes between 4.8 and 4.9, had an impact on the metrics computed at each iteration inducing different results between the two versions. Maybe would be good to have such information somewhere in the documentation (in the FAQ? something related to compatibility across versions maybe?). Thanks once again for your quick answers and support! All your replies have been very helpful. Thank you all |
elastix 4.9 has worse performance than 4.8.
Part of the problem is fixed in this commit:
eab944a
For the combo metric the problem persists, yielding registration times of 5 minutes in elastix 4.9 while being 1.5 minutes in elastix 4.8.
Recompiling with ITK 4.8.2 instead of ITK 4.13 showed no degradation, leading to the conclusion that a change in ITK inbetween those two versions caused the performance degradation.
The text was updated successfully, but these errors were encountered: