(fix): Close the right boundary of WrapExtrap's domain#142
Merged
Conversation
Switch from half-open [first(x), last(x)) to closed [first(x), last(x)]. xq == last(x) now returns xi unchanged (was wrap-to-first). :inclusive PeriodicBC unchanged (validation enforces y[1] = y[end]). :exclusive PeriodicBC unchanged (seam search returns idx_R=1 at xq>=inner[n]). Plain WrapExtrap at xq==last(x) now returns y[end] (was y[1]).
…osed Union
Phase 1.2 of WrapExtrap closed-domain conversion. WrapExtrap batch
dispatch now shares the closed-domain promotion with ClampExtrap and
FillExtrap via one Union{Clamp,Fill,Wrap} method.
Also fixes a latent bug in _constant_eval_at_anchor's right-edge
short-circuit: y[end] was correct under half-open _wrap_to_domain
(which wrapped seam queries to x_min before this point fired), but
returns wrong value y[n] instead of cyclic y[1] under closed semantics
on _ExclusivePeriodicAxis (where x_last is the virtual seam endpoint).
Routes through y[aq.idxR] which equals y[end] for non-periodic
(idxR == n) and y[1] for :exclusive PeriodicBC (idxR == 1 post-fold).
Phase 2 of WrapExtrap closed-domain conversion. _anchor_loc's wrap branch now skips at exact xq==x_max (the inner _wrap_to_domain already became a no-op there under Phase 1.1; this is the cleanup for code clarity). Test test_anchor_common.jl 'wrap at x_max' rewritten: query at x_max now asserts in-domain state at the last cell (idx==n-1, xR==x_max), not wrap-to-x_min. New test guards strictly-OOB right still wraps.
Phase 3 of WrapExtrap closed-domain conversion. The slow path already handled qmax==x_max correctly under closed semantics (since _wrap_to_domain is a no-op there); this change moves it to the fast path to skip the per-element wrap overhead. Adds skeleton test_wrapextrap_closed_boundary.jl (Phase 5 will expand to full method matrix).
Replace half-open form '[x[1], x[end])' / '[x_min, x_max)' in docstrings
and comments across:
- src/core/eval_ops.jl (WrapExtrap docstring)
- src/constant/constant_oneshot.jl (public docstring)
- src/cubic/cubic_adjoint.jl, cubic/cubic_anchor.jl (anchor doc)
- src/{linear,constant,quadratic}/{,linear,constant,quadratic}_anchor.jl
- test_periodic_bc.jl (NOTE on Linear WrapExtrap endpoint validation)
- test_constant_periodic.jl (seam-search explanation)
- test_hermite_1d.jl (tighten 'fast-path includes x_max' to literal assert)
linear/nd/linear_nd_adjoint.jl 'half-open grid' refers to user input data
layout for :exclusive PeriodicBC (n samples covering [first, first+period)),
not the WrapExtrap domain — unchanged.
Phase 5 of WrapExtrap closed-domain conversion. test_wrapextrap_closed_boundary.jl expands the Phase 3 stub into a full coverage matrix: - Linear / Constant / Cubic / Quadratic 1D × every extrap × Vector + Range grid - Hermite (precomputed dy) + OnTheFly Hermite (PCHIP / Cardinal / Akima) - :inclusive PeriodicBC at xq = last(x) — verify y[1] (== y[end] by validation) - :exclusive PeriodicBC at xq = inner[1]+period — verify y[1] via seam search - :exclusive series oneshot — regression guard for the constant-anchor short- circuit fix (was returning y[end] cyclically instead of y[idxR=1]) - ND per-axis WrapExtrap at corner (last(gx), last(gy)) — scalar + SoA batch
Tier 2 regression caught 7 failures in test_constant.jl asserting the old half-open WrapExtrap behavior at xq == last(x). Investigation showed that constant scalar oneshot _constant_eval_at_point(::WrapExtrap) was the only constant path lacking the 'xq == last(x) -> y[end]' short-circuit that the InBounds core and the persistent anchor path both carry. Under old half-open semantics, this didn't matter because xq == last(x) got wrapped to first(x) before reaching the kernel. Under closed semantics (this PR) the query is preserved, and the kernel returned y[idx_L] for LeftSide / y[idx_R] for RightSide instead of the uniform y[end] the rest of the API uses at the right boundary. Add the short-circuit, mirroring the InBounds + anchor-path implementations. preserves cyclic semantics for :exclusive PeriodicBC. Updated 7 test_constant.jl assertions to reflect the new closed contract (and the consistent side-flag collapse at the boundary).
- Compress 4 copy-pasted 7-line right-edge short-circuit comments in constant_anchor.jl into one durable 2-line note per overload. - Drop `PR refac/wrap_closed` references and `claudedocs/TODO/` paths from src comments (per repo convention, src comments stay PR-agnostic). - Update `[first(x), last(x))` notation to `[first(x), last(x)]` in linear_oneshot.jl and test_periodic_bc.jl to match the closed domain.
…/ND hetero) Adds five new `@testitem` blocks to `test_wrapextrap_closed_boundary.jl` covering paths previously untested at `xq == last(x)` under WrapExtrap: - ND hetero corner: Cubic×Cubic, Quadratic×Quadratic, Cubic×Linear, Cardinal×Cardinal (OnTheFly cell-local path), and 3D Cubic×Cubic×Cubic. - Persistent (callable) interpolant at `last(x)` for every method family (Linear, Constant, Cubic, Quadratic, PCHIP, Cardinal, Akima, Hermite). - Adjoint at `last(x)` lands at `f_bar[end]`, not `f_bar[1]` (every adjoint family, including data-dependent OnTheFly slope adjoints). - Derivative at `last(x)` matches the last-segment slope (using asymmetric `exp(x)` data so first vs last-segment slopes differ). - Zero-alloc guard on the new short-circuits (constant scalar oneshot, linear batch in-place), gated by `ALLOC_THRESHOLD` for LTS slack.
Contributor
FastInterpolations.jl BenchmarksAll benchmarks (50 total, click to expand)
|
| Benchmark | Current | Previous | Imm. Ratio | Grad. Ratio | Tier |
|---|---|---|---|---|---|
1_cubic_oneshot/q00001 |
537.4 ns |
538.2 ns |
0.998 |
1.152 |
gradual |
7_cubic_range/scalar_query |
8.3 ns |
8.3 ns |
1.0 |
1.134 |
gradual |
8_cubic_multi/construct_s001_q100 |
637.8 ns |
637.8 ns |
1.0 |
1.122 |
gradual |
8_cubic_multi/eval_s001_q100 |
819.3 ns |
810.9 ns |
1.01 |
1.125 |
gradual |
9_nd_oneshot/bicubic_2d |
45526.2 ns |
44060.2 ns |
1.033 |
1.14 |
gradual |
9_nd_oneshot/tricubic_3d |
432014.8 ns |
424975.7 ns |
1.017 |
1.154 |
gradual |
Thresholds: immediate > 1.1x (vs latest master), gradual > 1.1x (vs sliding window)
This comment was automatically generated by Benchmark workflow.
There was a problem hiding this comment.
Pull request overview
This PR changes WrapExtrap seam handling so exact right-boundary queries are treated as in-domain ([first(x), last(x)]) rather than wrapping to the left edge, and updates tests/docs around that behavior.
Changes:
- Updates
_wrap_to_domain, batch domain checks, linear batch fast path, and constant right-edge handling to use closed-domain semantics. - Adjusts anchor and extrapolation documentation from half-open to closed-domain wording.
- Adds and updates tests covering boundary values, periodic invariants, ND corners, adjoints, derivatives, and allocation behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/core/periodic.jl |
Changes wrap fast path to include x_max and documents closed-domain behavior. |
src/core/utils.jl |
Makes WrapExtrap share the closed-domain batch InBounds promotion. |
src/core/eval_ops.jl |
Updates WrapExtrap public documentation for closed-domain semantics. |
src/core/anchor_common.jl |
Keeps exact x_max unwrapped during anchored query construction. |
src/constant/constant_oneshot.jl |
Adds right-edge short-circuit for scalar constant WrapExtrap. |
src/constant/constant_anchor.jl |
Uses anchor right index for right-edge constant anchor evaluation. |
src/linear/linear_oneshot.jl |
Allows linear batch fast path when qmax == x_max. |
src/linear/linear_anchor.jl |
Updates linear anchor docs for closed wrap domain. |
src/cubic/cubic_anchor.jl |
Updates cubic anchor docs/comments for closed wrap domain. |
src/cubic/cubic_adjoint.jl |
Updates periodic cubic adjoint wrap documentation. |
src/quadratic/quadratic_anchor.jl |
Updates quadratic anchor docs for closed wrap domain. |
test/test_wrapextrap_closed_boundary.jl |
Adds broad regression coverage for exact right-boundary WrapExtrap behavior. |
test/test_wrap_to_domain_closed.jl |
Adds focused _wrap_to_domain and _check_domain closed-boundary tests. |
test/test_periodic_bc.jl |
Updates periodic comments for closed-domain semantics. |
test/test_hermite_1d.jl |
Tightens Hermite right-boundary WrapExtrap expectation. |
test/test_constant.jl |
Updates constant interpolation seam expectations for closed WrapExtrap. |
test/test_constant_periodic.jl |
Updates exclusive periodic series seam comments/assertions. |
test/test_anchor_common.jl |
Updates anchor-location tests for no-wrap at exact x_max. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #142 +/- ##
==========================================
- Coverage 96.43% 96.43% -0.01%
==========================================
Files 143 143
Lines 12001 11994 -7
==========================================
- Hits 11573 11566 -7
Misses 428 428
🚀 New features to boost your workflow:
|
mgyoo86
referenced
this pull request
May 19, 2026
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
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.
Summary
WrapExtrap's in-domain interval is now closed:[first(x), last(x)]. Previously it was half-open[first(x), last(x)), so a query at exactlyxq == last(x)wrapped to the left edge and returnedy[1]. With closed semantics it stays at the right edge and returnsy[end].This is a behavioral change at exactly one point —
xq == last(x)under plainWrapExtrap. All other queries (xq < last(x),xq > last(x),xq < first(x)) are unchanged.PeriodicBCusers are observably unaffected::inclusiverequiresy[1] == y[end]by construction, so the new return value at the seam is equal to the old one.:exclusivereaches the seam through_ExclusivePeriodicAxis.search_interval, which already used an inclusivexq >= x[n]predicate — its fast path returns the cyclic value (y[1]) regardless of the wrap-predicate change.