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

Update elastix to April 19 2024 hash #2104

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

blowekamp
Copy link
Member

No description provided.

@blowekamp
Copy link
Member Author

@N-Dekker Even after this update I am seeming a large number of warning from Elastix. A lot of "unused typedefs", "overloaded virtual", "unused parameter" etc. Is this expected or does something need to be corrected with the build?

@N-Dekker
Copy link
Collaborator

A lot of "unused typedefs", "overloaded virtual", "unused parameter" etc. Is this expected or does something need to be corrected with the build?

Thanks for reporting, but I don't know! Maybe you use a different compiler version, or a different warning level. Can you give me the list of warnings with details? Are they at https://open.cdash.org/index.php?project=SimpleITK ?

@blowekamp
Copy link
Member Author

blowekamp commented Apr 24, 2024

I saw those locally with Apple clang 14.0.0. I don't believe the extra warning flag enable for SimpelITK are passed to the Elastix build.

The CI for this PR failed with a missing header:
I saw those local. But the CI for this PR failed with a missing header:
https://open.cdash.org/viewBuildError.php?buildid=9567908

"Elastix-prefix/include/Core/ComponentBaseClasses/elxFixedImagePyramidBase.h:26:10: fatal error: itkMultiResolutionPyramidImageFilter.h: No such file or directory"

@N-Dekker
Copy link
Collaborator

The CI for this PR failed with a missing header: I saw those local. But the CI for this PR failed with a missing header: https://open.cdash.org/viewBuildError.php?buildid=9567908

"Elastix-prefix/include/Core/ComponentBaseClasses/elxFixedImagePyramidBase.h:26:10: fatal error: itkMultiResolutionPyramidImageFilter.h: No such file or directory"

itkMultiResolutionPyramidImageFilter.h is here: https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Registration/Common/include/itkMultiResolutionPyramidImageFilter.h

So I think the compiler command-line for "Code/ElastixTransformixWrappers/src/sitkElastixImageFilterImpl.cxx" at https://open.cdash.org/viewBuildError.php?buildid=9567908 should have something like:

 -I/.../SimpleITK-build/ITK/Modules/Registration/Common/include

But I don't see it 🤷 Is it missing somehow?

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 24, 2024
Triggered by issue #1105 "Add macos-14 in CI" submitted by Matt McCormick, as well as a remark by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)
@blowekamp
Copy link
Member Author

blowekamp commented Apr 24, 2024

@N-Dekker I think the Elastix libraries did not express their build dependencies in a conventional way to they were explicitly add here: https://github.com/SimpleITK/SimpleITK/blob/master/Code/ElastixTransformixWrappers/src/CMakeLists.txt#L8 I am testing adding ITKRegistrationCommon now.

P.S. Additionally the warning are produced when using the Elastix libraries in SimpleITK with those additional warning flags enabled.

@N-Dekker
Copy link
Collaborator

@blowekamp I just tried to upgrade the elastix CI from macos-12 to macos-14: https://github.com/SuperElastix/elastix/tree/GitHubActions-macos-14

Unfortunately, it did not give me the warnings you mentioned at #2104 (comment) Instead, I just got:

ld: warning: ignoring duplicate libraries: '-lm', '/.../ITK-build/lib/libITKCommon-5.4.a', ...

at https://my.cdash.org/viewBuildError.php?type=1&buildid=2549153

Do you have a suggestion how I can get "your" elastix warnings, instead?

@blowekamp
Copy link
Member Author

The warnings are related to these flags:

-Wunused-parameter
-Wshadow
-Wdeprecated-declarations
-Woverloaded-virtual

Which I believe are being automatically added by: https://github.com/SimpleITK/SimpleITK/blob/7257d455d19bef2c13f696fbc2a4dc950685aacb/CMake/sitkCompilerWarningsSettings.cmake

@blowekamp blowekamp force-pushed the update_elastix branch 2 times, most recently from c5e4959 to b98e32f Compare April 24, 2024 22:38
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 25, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce the warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
@N-Dekker
Copy link
Collaborator

N-Dekker commented Apr 25, 2024

@blowekamp FYI, I'm preparing a PR to fix those warnings that you encountered: SuperElastix/elastix@main...Enable--Wshadow-Woverloaded-virtual

Wow, disappointing: I thought I fixed all 50 warnings from https://my.cdash.org/viewBuildError.php?type=1&buildid=2549592 but after pushing the fixes, it just produces 50 other warnings. Apparently it only shows 50 warnings at a time.


Ah, I see, at the elastix dashboard page https://my.cdash.org/viewBuildError.php?type=1&buildid=2549649

The maximum number of reported warnings or errors has been reached!!!

Update: I just found how to increase the maximum number of reported warnings, at

SET (CTEST_CUSTOM_MAXIMUM_NUMBER_OF_WARNINGS 999)
😃

@blowekamp
Copy link
Member Author

There are some now showing in the CI dashboard for this PR here: https://open.cdash.org/viewBuildError.php?type=1&buildid=9570768

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 25, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce the warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 25, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce the warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 26, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce the warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 26, 2024
Triggered by issue #1105 "Add macos-14 in CI" submitted by Matt McCormick, as well as a remark by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 26, 2024
Triggered by issue #1105 "Add macos-14 in CI" submitted by Matt McCormick, as well as a remark by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

macos-11 is no longer maintained anyway. It is now deprecated, according to https://learn.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 26, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 26, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 26, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
@N-Dekker
Copy link
Collaborator

@blowekamp I'm sorry to say the extra compiler flags (-Wunused-parameter -Wshadow -Wdeprecated-declarations -Woverloaded-virtual) produce more warnings than I expected. For -Wshadow alone, it's already 1000+: https://my.cdash.org/viewBuildError.php?type=1&buildid=2550567 So it takes me more time than expected 🤷 To be continued...

@blowekamp
Copy link
Member Author

@blowekamp I'm sorry to say the extra compiler flags (-Wunused-parameter -Wshadow -Wdeprecated-declarations -Woverloaded-virtual) produce more warnings than I expected. For -Wshadow alone, it's already 1000+: https://my.cdash.org/viewBuildError.php?type=1&buildid=2550567 So it takes me more time than expected 🤷 To be continued...

Thank you for working on this. Hopefully, you can find some tools to automate some of the cleanup.

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 29, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 30, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Apr 30, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Triggered by issue #1105 "Add macos-14 in CI" submitted by Matt McCormick, as well as a remark by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Triggered by issue #1105 "Add macos-14 in CI" submitted by Matt McCormick, as well as a remark by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

macos-11 is no longer maintained anyway. It is now deprecated, according to https://learn.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 7, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 8, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 8, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 9, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request May 9, 2024
Suggested by Bradley Lowekamp at SimpleITK/SimpleITK#2104 (comment)

Aims to reproduce those warnings from SimpleITK/monarch-i-06c02d82ab38530ea-Linux at https://open.cdash.org/viewBuildError.php?type=1&buildid=9568906
@N-Dekker
Copy link
Collaborator

N-Dekker commented Jun 4, 2024

@blowekamp It took a while, but I think I addressed all of those (2000+) warnings, with elastix pull request SuperElastix/elastix#1136 commit SuperElastix/elastix@78fe482, now at the main branch of https://github.com/SuperElastix/elastix Thanks for your patience 😃 Can you please give it another try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants