WIP: Add SpatialObject::WorldSpaceContext class, a faster alternative to SpatialObject::IsInsideInWorldSpace
#4434
Conversation
github-actions
bot
added
type:Testing
N-Dekker
force-pushed
the
WorldSpaceContext
branch
from
45cace9
to
c68c099
Compare
N-Dekker
changed the title
SpatialObject::WorldSpaceContext class, a faster alternative to SpatialObject::IsInsideInWorldSpaceSpatialObject::WorldSpaceContext class, a faster alternative to SpatialObject::IsInsideInWorldSpace
N-Dekker
force-pushed
the
WorldSpaceContext
branch
from
c68c099
to
c39d354
Compare
`WorldSpaceContext::IsInsideSpatialObject` serves as a faster alternative to the existing `SpatialObject::IsInsideInWorldSpace`. `WorldSpaceContext` has already cached the ObjectToWorldTransformInverse transform during its construction. `IsInsideSpatialObject` calls the faster `SpatialObject::IsInsideInObjectSpace(const PointType &)` overload, instead of the overload with three parameters. Using VS2022 Release, `WorldSpaceContext::IsInsideSpatialObject` appeared more than twice as fast as the equivalent `SpatialObject::IsInsideInWorldSpace`, taking less than 0.35 sec. for 2^25 WorldSpaceContext::IsInsideSpatialObject calls, while the same number of SpatialObject::IsInsideInWorldSpace calls took more than 0.85 sec. Included ImageMaskSpatialObject WorldSpaceContext unit test.
A major performance improvement is achieved by creating a `WorldSpaceContext` _outside_ of the pixel-by-pixel iteration loop, and calling `WorldSpaceContext::IsInsideSpatialObject` inside the loop, instead of `SpatialObject::IsInsideInWorldSpace`.
N-Dekker
force-pushed
the
WorldSpaceContext
branch
from
c39d354
to
169c290
Compare
| const typename Superclass::FixedImageMaskType::WorldSpaceContext worldSpaceContext(Superclass::m_FixedImageMask); | ||
|
|
||
| /** \todo multi-thread me */ | ||
| while (!fi.IsAtEnd()) | ||
| { |
There was a problem hiding this comment.
Should MinimumMaximumImageCalculator be used here? And parallelize it? Though parallelization might not matter much in this cases, as this is almost certainly limited by memory throughput, not computation.
There was a problem hiding this comment.
Thanks @dzenanz I guess it would be nice to use MinimumMaximumImageCalculator here indeed. If you want to do so, please go ahead 😃 I'm considering to abandon this PR (the introduction of SpatialObject::WorldSpaceContext) anyway. If we can sufficiently improve the speed of SpatialObject::IsInsideInWorldSpace (starting by pull request #4435), I don't think the introduction of SpatialObject::WorldSpaceContext is useful anymore.
There was a problem hiding this comment.
I recall optimizing the itk::MinimumMaximumImageFilter and was able to get surprising gains in the filter.
There was a problem hiding this comment.
As far as I can see, MinimumMaximumImageCalculator does not support masks: https://github.com/InsightSoftwareConsortium/ITK/blob/8010fafde3eb40a69db6ee9f49e547d0565da92b/Modules/Core/Common/include/itkMinimumMaximumImageCalculator.h
There was a problem hiding this comment.
Ah, I didn't think about that 😄
|
This pull request (#4434, proposing |
WorldSpaceContext::IsInsideSpatialObjectserves as a faster alternative to theexisting
SpatialObject::IsInsideInWorldSpace.WorldSpaceContexthas alreadycached the ObjectToWorldTransformInverse transform during its construction.
IsInsideSpatialObjectcalls the fasterSpatialObject::IsInsideInObjectSpace(const PointType &)overload, instead ofthe overload with three parameters.
Using VS2022 Release,
WorldSpaceContext::IsInsideSpatialObjectappeared morethan twice as fast as the equivalent
SpatialObject::IsInsideInWorldSpacecall.Update: If the
IsInsideInWorldSpace(const PointType &)overload proposed by pull request #4440 can make it onto the master branch, I will probably drop thisWorldSpaceContextproposal (#4434), because then I thinkWorldSpaceContext::IsInsideSpatialObject(point)won't be much faster than the equivalentSpatialObject::IsInsideInWorldSpacecall anymore.