-
Notifications
You must be signed in to change notification settings - Fork 27
ENH: Switch to latest version of SuperElastix/elastix (using ITK 5.1.1) #60
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
ENH: Switch to latest version of SuperElastix/elastix (using ITK 5.1.1) #60
Conversation
|
Follow-up to pull request #58 (which was accidentally closed) |
|
I remember there was some discussion about elastix building with |
Yes, Matt (@thewtex) wanted to be able to build elastix + ITK 5.1 with |
ce90513 to
8f934e9
Compare
Removed obsolete `elastix::BaseComponent::InitializeElastixLibrary()` calls, as Elastix is now initialized as a library by default: "ENH: Replace InitializeElastixLibrary() by InitializeElastixExecutable()" SuperElastix/elastix#248 SuperElastix/elastix@ef64fb2 Adjusted CMake `Elastix_LIBRARIES`, as the Elastix library CMake targets are now named elastix_lib and transformix_lib: "ENH: Allow building exe and lib of elastix and transformix together" SuperElastix/elastix#232 SuperElastix/elastix@3944a13
8f934e9 to
2bdca95
Compare
|
It looks like this PR run entirely fine now at GitHub Actions, but I still need to have a look at the OpenCL issue. I did not yet merge SuperElastix/elastix#213 "Khronos OpenCL support" from Matt (@thewtex) because I haven't been able to reproduce the issue so far. Would it be possible to have a test that demonstrates that this change, or thewtex/elastix@9eb50bc + thewtex/elastix@9b86123 are essential? Update: just opened for review! Would it be OK to just merge this PR now, and then possibly add Khronos OpenCL support later? For me it takes too much time now to properly test OpenCL support. |
|
I ran into compile errors in a local build with |
|
Thanks for merging, @dzenanz ! We certainly intended elastix to support |
|
Oh, the problem was that |
|
@dzenanz Do you really need to support For example, the error you got, "method with override specifier 'override' did not override":
The method certainly does override when |
|
Elastix does not need to support that. But I usually have it turned on, as I want the code I write to not use old style. That's how I noticed it. I can certainly keep it OFF when working with ITKElastix. |
Removed obsolete
elastix::BaseComponent::InitializeElastixLibrary()calls, as Elastix is now initialized as a library by default:
"ENH: Replace InitializeElastixLibrary() by InitializeElastixExecutable()"
SuperElastix/elastix#248
SuperElastix/elastix@ef64fb2
Adjusted CMake
Elastix_LIBRARIES, as the Elastix library CMake targetsare now named elastix_lib and transformix_lib:
"ENH: Allow building exe and lib of elastix and transformix together"
SuperElastix/elastix#232
SuperElastix/elastix@3944a13