ENH: Simpler TransformPhysicalPointToContinuousIndex(point) overloads (rebase of #4002)#6186
Closed
hjmjohnson wants to merge 3 commits intoInsightSoftwareConsortium:mainfrom
Conversation
github-actions
Bot
added
type:Enhancement
hjmjohnson
added a commit
to hjmjohnson/ITK
that referenced
this pull request
May 2, 2026
`Documentation/docs/releases/5.4.6.md` carries a trailing empty line that the `end-of-file-fixer` pre-commit hook flags every time a PR rebases onto current `main`. Affected at least PRs InsightSoftwareConsortium#6032, InsightSoftwareConsortium#4221, and InsightSoftwareConsortium#6186, where each had to carry an identical one-character fix. Apply once on `main` to stop the spurious `pre-commit` failures.
hjmjohnson
force-pushed
the
Add-TransformPhysicalPointToContinuousIndex-overload-rebased
branch
from
8da1845 to
6128fea
Compare
github-actions
Bot
removed
the
area:Documentation
hjmjohnson
force-pushed
the
Add-TransformPhysicalPointToContinuousIndex-overload-rebased
branch
from
6128fea to
de8c7ed
Compare
Member
Author
|
@greptileai review this draft before I make it official |
This comment was marked as resolved.
This comment was marked as resolved.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
hjmjohnson
force-pushed
the
Add-TransformPhysicalPointToContinuousIndex-overload-rebased
branch
from
de8c7ed to
9cdd48f
Compare
hjmjohnson
force-pushed
the
Add-TransformPhysicalPointToContinuousIndex-overload-rebased
branch
2 times, most recently
from
0bc3437 to
a140cff
Compare
Member
Author
|
/azp run |
Member
Author
|
Retriggering ITK.Linux.Python — the previous run failed in Post-job: Restore ccache with /azp run ITK.Linux.Python |
Member
Author
|
/azp run ITK.Linux.Python |
ITK.Linux.Python builds 14748 and 14751 (and presumably any
subsequent run) fail at the implicit `Cache@2` post-job step that
tars `CCACHE_DIR` for upload, with
tar: <archive>: Wrote only 4096 of 10240 bytes
tar: Error is not recoverable: exiting now
##[error]Process returned non-zero exit code: 2
after 10+ `Free disk space on / is lower than 5%` warnings. The
build/test phase itself is green; only the cache upload fails.
The local ccache fills its 5G default ceiling during the build:
Cache size (GB): 4.94 / 5.00 (98.87 %)
Cleanups: 15
and the runner's writable disk is already 96% full when `tar` starts,
leaving no room for the staging archive. Worse, because the upload
fails every run, the pipeline cache key reports a miss every time, so
the next run is also cold and refills the local store from scratch.
Insert a between-step that runs after `Build and test` and before the
implicit post-job `Restore ccache` upload:
- `df -h /` for visibility on both sides of the cleanup;
- `ccache --evict-older-than 5d` to drop stale entries;
- `ccache --max-size 6.5G` to lower the soft ceiling;
- `ccache -c` to force the store under that ceiling;
- `ninja -C <build> clean` to free the .o files that are no longer
needed once the build/test phase has reported (`|| true` so a
missing/already-clean tree does not fail the step).
Setting max-size to 6.5G (above the in-build CCACHE_MAXSIZE=8G env
but below the runner's free-disk headroom) gives ccache room to
operate during the build while keeping the post-job tar inside the
runner's disk budget.
Added non-template overloads to `ImageBase`, `PhasedArray3DSpecialCoordinatesImage`,
and `ImageAdaptor`, which allow the user to write:
index = image->TransformPhysicalPointToContinuousIndex(point);
Instead of having to spell out the explicit template argument:
index = image->template TransformPhysicalPointToContinuousIndex<typename ContinuousIndexType::ValueType>(point);
The new overloads take the image's `PointType` (= `Point<SpacePrecisionType, N>`)
and return `ContinuousIndex<SpacePrecisionType, N>` — the dominant use case.
Users who need a different precision can still call the templated overload
explicitly.
Co-Authored-By: Hans Johnson <hans-johnson@uiowa.edu>
Replace `->template TransformPhysicalPointToContinuousIndex<X>(point)` with the new non-template overload `->TransformPhysicalPointToContinuousIndex(point)` at internal call sites where the explicit template parameter resolved to `itk::SpacePrecisionType` (i.e. the dominant case). Files updated cover ImageAlgorithm, TriangleMeshToBinaryImageFilter, BSplineBaseTransform, WarpImageFilter, the Registration Common metrics, and SLICImageFilter. Excludes unit tests in itkImageBaseGTest.cxx and itkImageAdaptorGTest.cxx which intentionally exercise the templated form. Co-Authored-By: Hans Johnson <hans-johnson@uiowa.edu>
hjmjohnson
force-pushed
the
Add-TransformPhysicalPointToContinuousIndex-overload-rebased
branch
from
a140cff to
20865f1
Compare
github-actions
Bot
added
type:Infrastructure
Member
Author
|
Rebased onto #6199 ( Once #6199 merges, I'll rebase this PR back onto plain |
Contributor
|
I'm sorry but I think the extra overloads may introduce more confusion. It seemed like a good idea in the beginning, but it is of very limited use. It just isn't obvious what is the preferred scalar type of the return type of |
Member
Author
|
@N-Dekker Thank you for the feedback. I'll close these to clean up the PR list. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased and finished version of #4002 (originally by @N-Dekker, April 2023). Adds non-template
TransformPhysicalPointToContinuousIndex(point)overloads onImageBase,PhasedArray3DSpecialCoordinatesImage, andImageAdaptor, then converts internal call sites to the simpler form.Why this supersedes #4002
#4002's branch was 4838 commits behind
upstream/main. A direct cherry-pick reverted two years of style modernization (itkExceptionStringMacro, trailing-returnauto, addedconsts, header reorganization) on the same files the STYLE commit touched. Rather than fight that, this PR is a clean re-implementation of N-Dekker's design on top of currentmain:COMP:commit (addingtemplatekeyword in tests) was already merged upstream — dropped.ENH:overload additions were re-applied to the three header files at their current upstream locations.STYLE:call-site replacements were re-applied via regex;CoordRepTypereferences on the touched lines were updated toCoordinateType(ITK 6 deprecated the old name).Open design question from #4002 — do we also want a non-template float overload?
@N-Dekker raised this in an inline comment on #4002: should we also offer
@dzenanz's reply: the float overload is rarely used; users who need it can spell out the explicit template parameter. This PR follows that guidance — only the
SpacePrecisionType-based overload is added. Users needingfloatprecision can still call the templated form explicitly.ITK_USE_FLOAT_SPACE_PRECISION compatibility
When
ITK_USE_FLOAT_SPACE_PRECISION=ON,SpacePrecisionType=float, so the new overload returnsContinuousIndex<float, N>. At call sites that assign toContinuousIndex<double, N>(e.g.WarpImageFilter,ImageToImageMetric), theContinuousIndexconverting constructor handles the widening. At sites that assign toContinuousIndex<CoordinateType, N>whereCoordinateType==SpacePrecisionType==float, the assignment is exact. Confirmed by @hjmjohnson in a 2025-09-16 comment on #4002 thatITK_USE_FLOAT_SPACE_PRECISIONstill compiles cleanly.Closes #4002.