ENH: Replace elastix ImageSpatialObject2 by ITK ImageSpatialObject #147

N-Dekker · 2019-05-07T13:30:14Z

Replaced elastix specific ImageSpatialObject2 and ImageMaskSpatialObject2 by ITK's ImageSpatialObject and ImageMaskSpatialObject, respectively.

Let azure CI build with the ITK5 version of May 9, 2019 (from ITK git master branch).

Added Mask->Update() calls to MultiResolutionRegistration::UpdateMasks, to ensure that the bounding boxes are computed.

Part of the elastix upgrade to ITKv5.

N-Dekker · 2019-05-07T16:48:17Z

Everything compiled well on all platforms. Only some CTest Elastix test failures on Windows ITKv5. https://dev.azure.com/kaspermarstal/Elastix/_build/results?buildId=414

Could this be just a hick up of the machine? I'll try to trigger a rebuild.

mstaring · 2019-05-08T09:14:01Z

looks great!

Where you able to check that the registrations (with masks) return identical results?
That should be done before merging this PR.

N-Dekker · 2019-05-09T09:32:05Z

Just updated ITKv5 SHA at the CI system, to ensure that this commit is included: InsightSoftwareConsortium/ITK@7b4717d "BUG: Half a pixel margin ImageMaskSpatialObject::ComputeMyBoundingBox()". I certainly won't merge when any test still fails at https://dev.azure.com/kaspermarstal/Elastix !

mstaring · 2019-05-09T13:07:35Z

Also, I am not sure we test registrations with a mask, so please check that we actually check for this functionality.

N-Dekker · 2019-05-13T11:05:49Z

CTest Elastix test failures on Windows ITKv5:
91 - elastix_run_3DCT_lung.MI.bspline.SGD.004_OUTPUT (Failed)
92 - elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_OVERLAP (Failed)
93 - elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_LANDMARKS (Failed)
94 - elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_TP (Failed)

N-Dekker · 2019-05-21T13:54:45Z

Still not OK, after the last force-push:

1> Start 19: elastix_run_example_OUTPUT
1> 19/158 Test #19: elastix_run_example_OUTPUT .............................................. Passed 5.02 sec
1> Start 20: elastix_run_example_COMPARE_IM
1> 20/158 Test #20: elastix_run_example_COMPARE_IM ..........................................***Failed 0.40 sec

N-Dekker · 2019-05-24T12:37:33Z

Closed and reopened, just to trigger rebuild

N-Dekker · 2019-05-24T14:12:00Z

So https://dev.azure.com/kaspermarstal/Elastix/_build/results?buildId=422 now says, for Windows ITKv5:

The following tests FAILED:
Errors while running CTest
92 - elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_OVERLAP (Failed)
94 - elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_TP (Failed)

Details:

test 92
Start 92: elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_OVERLAP

92: Test command: C:\hostedtoolcache\windows\Python\3.6.8\x64\python.exe "D:/a/1/s/Testing/elx_compare_overlap.py" "-b" "D:/a/1/Elastix-build/Testing/TransformParameters_3DCT_lung.MI.bspline.SGD.004.txt" "-d" "D:/a/1/Elastix-build/Testing/elastix_run_3DCT_lung.MI.bspline.SGD.004" "-m" "D:/a/1/s/Testing/Data/3DCT_lung_followup_mask.mha" "-p" "D:/a/1/Elastix-build/bin/Release/elxComputeOverlap.exe"
92: Test timeout computed to be: 1500

test 94
Start 94: elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_TP

94: Test command: D:\a\1\Elastix-build\bin\Release\elxTransformParametersCompare.exe "-base" "D:/a/1/Elastix-build/Testing/TransformParameters_3DCT_lung.MI.bspline.SGD.004.txt" "-test" "D:/a/1/Elastix-build/Testing/elastix_run_3DCT_lung.MI.bspline.SGD.004/TransformParameters.0.txt" "-a" "1e-3"
94: Test timeout computed to be: 1500
94: Baseline transform parameters: D:/a/1/Elastix-build/Testing/TransformParameters_3DCT_lung.MI.bspline.SGD.004.txt has 51300 parameters.
94: Test transform parameters: D:/a/1/Elastix-build/Testing/elastix_run_3DCT_lung.MI.bspline.SGD.004/TransformParameters.0.txt has 51300 parameters.
94: The norm of the difference between baseline and test transform parameters was computed, using
94: || baseline - test ||
94: ---------------------
94: || baseline ||
94: Computed difference: 72.3165 / 201.03 = 0.35973
94: Allowed difference: 0.001
94: ERROR: The difference is larger than acceptable!

N-Dekker · 2019-06-07T15:39:11Z

It appears that the differences between test results of this pull request versus the develop branch (as seen in Testing\elastix_run_3DCT_lung.MI.bspline.SGD.004\elastix.log) are gone when the following IsInside check (which should be redundant) is removed from the develop branch:

elastix/Common/itkImageMaskSpatialObject2.hxx

Lines 72 to 75 in 0a87c9d

    
           if( !this->GetBounds()->IsInside( point ) ) 
        
           { 
        
             return false; 
        
           }

The ImageFullSampler of elastix (develop branch) converts the index of a candidate sample to a point by calling ITK's ImageBase::TransformIndexToPhysicalPoint:

elastix/Common/ImageSamplers/itkImageFullSampler.hxx

Line 115 in aa26aa5

inputImage->TransformIndexToPhysicalPoint( index,

On the other hand, elastix ImageMaskSpatialObject2 estimates the corner points of the bounding box using ITK's MatrixOffsetTransformBase::TransformPoint:

elastix/Common/itkImageMaskSpatialObject2.hxx

Line 326 in aa26aa5

    
           PointType transformedPoint = this->GetIndexToWorldTransform()->TransformPoint( *itC );

Some very small rounding differences between the result of ImageBase::TransformIndexToPhysicalPoint and MatrixOffsetTransformBase::TransformPoint appear to cause the problem here! This is actually why I wanted to get rid of the redundant GetBounds()->IsInside(point) check. It's a source of rounding errors! Accepted for ITK5.0.0: InsightSoftwareConsortium/ITK#751

ITK's ImageBase::TransformIndexToPhysicalPoint: https://github.com/InsightSoftwareConsortium/ITK/blob/eb431fa16ae8b3a93e65db1f77e57e73dc83c6a2/Modules/Core/Common/include/itkImageBase.h#L476,L489

ITK's MatrixOffsetTransformBase::TransformPoint: https://github.com/InsightSoftwareConsortium/ITK/blob/eb431fa16ae8b3a93e65db1f77e57e73dc83c6a2/Modules/Core/Transform/include/itkMatrixOffsetTransformBase.hxx#L179,L183

mstaring · 2019-06-11T07:17:15Z

ok great, when with that gone, the results are identical, I have faith we have a correct implementation of the masks and bounding boxes now. Then we can go ahead with updating baselines and merging the new code.

The check if not `GetBounds()->IsInside(point)` appears to make `ImageFullSampler` sensitive to rounding errors. These rounding errors are caused by a subtle difference between ITK's `ImageBase::TransformIndexToPhysicalPoint` (called by elastix ImageFullSampler) and ITK's `MatrixOffsetTransformBase::TransformPoint` (called by elastix ImageMaskSpatialObject2). This commit should fix issue #153 "ImageFullSampler with ImageMaskSpatialObject2 misses some pixels" Related to pull request #147 "ENH: Replace elastix ImageSpatialObject2 by ITK ImageSpatialObject"

N-Dekker · 2019-06-12T12:43:33Z

@mstaring Do you agree that this pull request (which removes the old elastix ImageSpatialObject2 and itkImageMaskSpatialObject2) is ready to merge? It still has regression failures, but only when using ITK4. Which is basically elastix issue #153 (but then coming from ITK4 ImageMaskSpatialObject::IsInside)

mstaring · 2019-06-13T07:21:31Z

Common/GTesting/itkImageMaskSpatialObject2GTest.cxx

+#if ITK_VERSION_MAJOR < 5

 // First include the header file to be tested:
 #include "itkImageMaskSpatialObject2.h"


this does not work anymore if this file is deleted right?

Good point @mstaring ! I think the entire ImageMaskSpatialObject2 test should be removed, unconditionally. It wasn't such an interesting test anyway. Just my first attempt to get GoogleTest working with elastix.

mstaring · 2019-06-13T07:22:05Z

i have one comments, otherwise good to go

N-Dekker · 2019-06-13T09:55:00Z

@mstaring I would like to keep ITK4 on the dashboard until the upgrade to ITK5 is finalized. But now (with this pull request) ITK4 has two test failures:

elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_OVERLAP
elastix_run_3DCT_lung.MI.bspline.SGD.004_COMPARE_TP

These can probably be fixed by either using the previous baseline (TransformParameters_3DCT_lung.MI.bspline.SGD.004.txt.in) for ITK4, or by just excluding these particular tests, for ITK4. Do you have a suggestion?

mstaring · 2019-06-13T12:20:20Z

Just exclude the test, ITK4 is not worth the trouble anymore ;-)

Replaced elastix specific `ImageSpatialObject2` and `ImageMaskSpatialObject2` by ITK's `ImageSpatialObject` and `ImageMaskSpatialObject`, respectively. Let azure CI build with the ITK5 version of May 9, 2019 (from ITK git master branch). Added Mask->Update() calls to `MultiResolutionRegistration::UpdateMasks`, to ensure that the bounding boxes are computed. Removed elementary GoogleTest for `ImageMaskSpatialObject2`. Removed failing 3DCT_lung.MI.bspline.SGD.004 test for ITK4 (it passes on ITK5) Part of the elastix upgrade to ITKv5.

N-Dekker closed this May 7, 2019

N-Dekker reopened this May 7, 2019

N-Dekker force-pushed the Replace-ImageMaskSpatialObject2 branch from 5b64fa7 to 39378b8 Compare May 9, 2019 09:26

N-Dekker mentioned this pull request May 10, 2019

ImageSpatialObject2 data member cleanup #131

Closed

N-Dekker force-pushed the Replace-ImageMaskSpatialObject2 branch 2 times, most recently from 6814204 to 7ceff07 Compare May 21, 2019 11:49

N-Dekker closed this May 24, 2019

N-Dekker reopened this May 24, 2019

N-Dekker mentioned this pull request Jun 8, 2019

ImageFullSampler with ImageMaskSpatialObject2 misses some pixels #153

Closed

N-Dekker mentioned this pull request Jun 11, 2019

BUG: Remove bounds check from ImageMaskSpatialObject2 IsInside #154

Merged

N-Dekker force-pushed the Replace-ImageMaskSpatialObject2 branch from 7ceff07 to 52b1391 Compare June 11, 2019 17:09

mstaring reviewed Jun 13, 2019

View reviewed changes

N-Dekker force-pushed the Replace-ImageMaskSpatialObject2 branch from 52b1391 to 7c2523a Compare June 13, 2019 12:37

N-Dekker merged commit 475993e into develop Jun 13, 2019

N-Dekker deleted the Replace-ImageMaskSpatialObject2 branch June 19, 2019 15:07

ENH: Replace elastix ImageSpatialObject2 by ITK ImageSpatialObject #147

ENH: Replace elastix ImageSpatialObject2 by ITK ImageSpatialObject #147

Uh oh!

Conversation

N-Dekker commented May 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

N-Dekker commented May 7, 2019

Uh oh!

mstaring commented May 8, 2019

Uh oh!

N-Dekker commented May 9, 2019

Uh oh!

mstaring commented May 9, 2019

Uh oh!

N-Dekker commented May 13, 2019

Uh oh!

N-Dekker commented May 21, 2019

Uh oh!

N-Dekker commented May 24, 2019

Uh oh!

N-Dekker commented May 24, 2019

Uh oh!

N-Dekker commented Jun 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mstaring commented Jun 11, 2019

Uh oh!

N-Dekker commented Jun 12, 2019

Uh oh!

mstaring Jun 13, 2019

Choose a reason for hiding this comment

Uh oh!

N-Dekker Jun 13, 2019

Choose a reason for hiding this comment

Uh oh!

mstaring commented Jun 13, 2019

Uh oh!

N-Dekker commented Jun 13, 2019

Uh oh!

mstaring commented Jun 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

N-Dekker commented May 7, 2019 •

edited

Loading

N-Dekker commented Jun 7, 2019 •

edited

Loading