Consolidate interpolator #71
Conversation
thewtex
force-pushed
the
ConsolidateInterpolator
branch
from
23cfe3f
to
5a6fddd
Compare
|
Rebased |
voxel sizes and triggering the case in which some arrival image voxels that contribute to gradient calculations have illegal values.
structure is all over the place. The new interpolator will only include neighbouring voxels in the calculation if they meet requirements defined in a user supplied function. The complication is that interpolators are used at a number of stages in this code. One point is directly in the cost function, the other is in the PhysicalCentralDifferenceImageFunction. There's currently no way to set the one in PhysicalCentralDifferenceImageFunction. It probably makes sense for the cost function interpolator to be used by PhysicalCentralDifferenceImageFunction.
Rather than setting a target offset time, which is tricky to set correctly unless the speed function is very flat (which is unrealistic). This patch ensures that a neighborhood around each target point is included as target points, ensuring that the neighbourhood arrival time is correctly computed. The patch also fixes a bug that meant that only one target point was included from each set (i.e extended seeds weren't working properly).
There was a problem hiding this comment.
Rebased on 164c017, see https://github.com/romangrothausmann/ITKMinimalPathExtraction/tree/ConsolidateInterpolator
@thewtex should I force push to your thewtex:ConsolidateInterpolator?
The space changes can really drive one mad, hope I did not break anything while solving the merge conflicts. @richardbeare what do you think?
|
@romangrothausmann great! Yes, please go ahead and force-push. |
…ction For: /home/matt/src/ITK/Modules/Core/ImageFunction/include/itkInterpolateImageFunction.h:150:3: note: unimplemented pure virtual method 'GetRadius' in 'LinearInterpolateSelectedNeighborsImageFunction' GetRadius is a pure virtual in ITK 5.
romangrothausmann
force-pushed
the
ConsolidateInterpolator
branch
from
d7a17a6
to
1c4e7c1
Compare
|
Tested this with the same CLI and get the same results as with the implementation provided by @richardbeare for #61: |
|
@romangrothausmann awesome! @richardbeare would be wonderful to get your review. |
|
On the to-do list |
There was a problem hiding this comment.
I think you have covered everything I tried to add. I can see the commits relating to an old bug with extended seeds, ensuring that the arrival times of neighbours of target points are evaluated, automated estimation of stopping conditions as well as the special interpolator.
I can't remember experimenting with anything else, so I think it is good to go.
|
It looks like some test baselines need to be updated and other tests are not generating baselines. @romangrothausmann do all tests pass locally? |
Sorry, only tested with my CLIs and the expected results, I guess the tests need to be updated to new standards of ITK-5 as well. |
No description provided.