Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ENH: Bump for ITK v5.4rc01 #236

Merged
merged 1 commit into from
Jul 24, 2023
Merged

ENH: Bump for ITK v5.4rc01 #236

merged 1 commit into from
Jul 24, 2023

Conversation

tbirdso
Copy link
Collaborator

@tbirdso tbirdso commented Jul 24, 2023

Bump CI and package dependencies for ITK v5.4 release candidate 1.

Expected to include update to C++17 build tools.

Bump CI and package dependencies for ITK v5.4 release candidate 1.

Expected to include update to C++17 build tools.
@tbirdso tbirdso merged commit 0d17498 into main Jul 24, 2023
@N-Dekker
Copy link
Collaborator

Thanks @tbirdso, really cool! Hope it fixes the CI for PR #236

@thewtex thewtex deleted the bump-itk-v5.4rc01 branch July 26, 2023 22:13
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Aug 25, 2023
Upgraded ITK on the CI to version 5.4rc01, which was released by Matt McCormick on 28 July 2023: https://github.com/InsightSoftwareConsortium/ITK/releases/tag/v5.4rc01

Following ITKElastix pull request InsightSoftwareConsortium/ITKElastix#236 commit InsightSoftwareConsortium/ITKElastix@c711bbd "ENH: Bump for ITK v5.4rc01", Tom Birdsong, July 24, 2023.

Follow-up to pull request #762 commit 353a095 "COMP: Upgrade ITK from v5.3rc04 to v5.3.0", August 25, 2022.

ITK v5.4rc01 includes an upgrade from C++14 to C++17.
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Aug 26, 2023
Upgraded ITK on the CI to version 5.4rc01, which was released by Matt McCormick on 28 July 2023: https://github.com/InsightSoftwareConsortium/ITK/releases/tag/v5.4rc01

Following ITKElastix pull request InsightSoftwareConsortium/ITKElastix#236 commit InsightSoftwareConsortium/ITKElastix@c711bbd "ENH: Bump for ITK v5.4rc01", Tom Birdsong, July 24, 2023.

Follow-up to pull request #762 commit 353a095 "COMP: Upgrade ITK from v5.3rc04 to v5.3.0", August 25, 2022.

ITK v5.4rc01 includes an upgrade from C++14 to C++17.
@N-Dekker
Copy link
Collaborator

I'm just trying to upgrade ITK from v5.3.0 to v5.4rc01 at the SuperElastix/elastix CI as well: https://github.com/SuperElastix/elastix/tree/Upgrade-ITK-to-v5.4rc01 However, to my surprise, it appears to trigger some regression failures:

From both Windows and Ubuntu, at https://dev.azure.com/kaspermarstal/Elastix/_build/results?buildId=4292&view=results

2023-08-26T17:06:52.3783895Z The following tests FAILED:
2023-08-26T17:06:52.3784191Z 	 54 - elastix_run_3DCT_lung.Kappa.bspline.ASGD.001_COMPARE_OVERLAP (Failed)
2023-08-26T17:06:52.3784590Z 	 55 - elastix_run_3DCT_lung.Kappa.bspline.ASGD.001_COMPARE_LANDMARKS (Failed)
2023-08-26T17:06:52.3784910Z 	 83 - elastix_run_3DCT_lung.MI.bspline.SGD.001_COMPARE_TP (Failed)
2023-08-26T17:06:52.3785238Z 	131 - elastix_run_3DCT_lung.MI.bspline.SGD.001-Threads1_COMPARE_TP (Failed)
2023-08-26T17:06:52.3785577Z 	135 - elastix_run_3DCT_lung.MI.bspline.SGD.001-Threads2_COMPARE_TP (Failed)
2023-08-26T17:06:52.3785913Z 	139 - elastix_run_3DCT_lung.MI.bspline.SGD.001-Threads4_COMPARE_TP (Failed)

And from macOS, at https://my.cdash.org/viewTest.php?onlyfailed&buildid=2395262

6 tests failed.
Name 	Status 	Time 	Details 	Summary
elastix_run_3DCT_lung.Kappa.bspline.ASGD.001_COMPARE_LANDMARKS 	Failed 	390ms 	Completed (Failed) 	Unstable
elastix_run_3DCT_lung.Kappa.bspline.ASGD.001_COMPARE_OVERLAP 	Failed 	6s 870ms 	Completed (Failed) 	Unstable
elastix_run_3DCT_lung.MI.bspline.SGD.001-Threads1_COMPARE_TP 	Failed 	160ms 	Completed (Failed) 	Unstable
elastix_run_3DCT_lung.MI.bspline.SGD.001-Threads2_COMPARE_TP 	Failed 	150ms 	Completed (Failed) 	Unstable
elastix_run_3DCT_lung.MI.bspline.SGD.001-Threads4_COMPARE_TP 	Failed 	150ms 	Completed (Failed) 	Unstable
elastix_run_3DCT_lung.MI.bspline.SGD.001_COMPARE_TP 	Failed 	90ms 	Completed (Failed) 	Unstable

Details from https://my.cdash.org/test/90114840 (elastix_run_3DCT_lung.Kappa.bspline.ASGD.001_COMPARE_LANDMARKS):

Transforming fixed image landmarks using /Users/runner/work/elastix/elastix/Elastix-build/Testing/elastix_run_3DCT_lung.Kappa.bspline.ASGD.001/TransformParameters.0.txt
Transforming fixed image landmarks using /Users/runner/work/elastix/elastix/Elastix-build/Testing/TransformParameters_3DCT_lung.Kappa.bspline.ASGD.001.txt
The landmark distance between current and baseline is:
min   | Q1    | med   | Q3    | max   | mean
0.000 | 0.000 | 0.504 | 1.135 | 5.791 | 0.755
FAILURE: third quartile landmark distance is higher than 1.0 mm

And from https://my.cdash.org/test/90114841 (elastix_run_3DCT_lung.Kappa.bspline.ASGD.001_COMPARE_OVERLAP):

Deforming moving image segmentation using /Users/runner/work/elastix/elastix/Elastix-build/Testing/elastix_run_3DCT_lung.Kappa.bspline.ASGD.001/TransformParameters.0.txt
Deforming moving image segmentation using /Users/runner/work/elastix/elastix/Elastix-build/Testing/TransformParameters_3DCT_lung.Kappa.bspline.ASGD.001.txt
The segmentation overlap between current and baseline is 0.973903

FAILURE: overlap is lower than 0.99

I do not know if these differences are of any importance, I mean I don't know if these are really buggy, but they certainly suggest that the behavior has changed from ITK v5.3.0 to v5.4rc01. Please let me know if you have a clue about those changes!

@N-Dekker
Copy link
Collaborator

N-Dekker commented Sep 2, 2023

I do not know if these differences are of any importance, I mean I don't know if these are really buggy, but they certainly suggest that the behavior has changed from ITK v5.3.0 to v5.4rc01.

Update: I found that the behavior change between ITK v5.3.0 to v5.4rc01 was caused by pull request InsightSoftwareConsortium/ITK#3929 commit InsightSoftwareConsortium/ITK@4e46cb6 "STYLE: Default default-constructor of ImageRandom ConstIterator classes", merged on February 24, 2023 (My 😧 fault!) It should be fixed (restoring the v5.3.0 behavior) by InsightSoftwareConsortium/ITK#4183 "BUG: ImageRandomIteratorWithIndex should not assign data in constructor". I hope this fix can still be included with the next ITK release (candidate).

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

Successfully merging this pull request may close these issues.

3 participants