refactor(phs): one `log_reference` keyword for the log transform by mgyoo86 · Pull Request #209 · ProjectTorreyPines/FastInterpolations.jl

mgyoo86 · 2026-10-05T19:21:12Z

Follow-up to #136.

Summary

The PHS log-density transform (interpolate ln(ρ/ρ₀), return ρ₀·G) is now set with one keyword, log_reference: nothing (default, off), a nonzero constant, or a callable ρ₀(q). The exported ConstantRef and PHSLogTransform and the keywords reference_interp / reference_data are removed. PHS is not in a release yet (merged after v0.4.19), so there is no deprecation path.

# before
phs_interp(grids, ρ; reference_interp = ConstantRef(1.0))
phs_interp(grids, ρ; reference_interp = ref)
phs_interp(grids, ρ; reference_interp = itp₀, reference_data = ρ₀)   # itp₀: PHS of ρ₀ on the same grid

# after
phs_interp(grids, ρ; log_reference = 1.0)   # any nonzero constant with the sign of ρ, used as given
phs_interp(grids, ρ; log_reference = ref)   # an interpolant, or a callable ρ₀(q) / ρ₀(q; deriv = ops)
phs_interp(grids, ρ; log_reference = 1.0)   # replaces the nested form: same result, see below

The public PHS surface is now phs_interp, phs_interp! and PHSInterpolantND.

Behavior changes

Validation at build, at grid precision: every ratio ρ/ρ₀ must be positive and finite, and the error names the node. This rejects zero or mixed-sign data, a reference of the wrong sign, Bool, arrays, and constants that vanish at grid precision (Float32 grid with 1e-100 gave NaN). Master stored -Inf, threw a bare DomainError/MethodError, or returned NaN.
reference_data is dropped: its use was the nested form above, where the reference is a PHS of ρ₀ on the data grid. That reference cancels itself in the local PHS operator, so the result equals the constant reference (differences ~1e-8 of the values, far below the interpolation error). A reference only adds accuracy with information the grid lacks: an analytic ρ₀, or an interpolant of finer data.
Docs: the log-transform derivative formulas now match the code (ρ̃ = ρ₀G with Leibniz terms), and the example uses positive data (it produced NaN).

Unchanged

Every case both versions support gives bit-identical results (2-D and 3-D; plain, constant, callable and interpolant references; values, first and second derivatives; one-shot calls). On the same queries, evaluation runs at 0.93× master's time (geomean of 55 pairs, none slower by more than 1 %), and builds are within run-to-run noise.
A callable reference is called as on master: ρ₀(q) for values, ρ₀(q; deriv = ops) for derivatives, and once per grid node at build. The ForwardDiff extension is untouched.
The one-shot blend_factor default stays 2.0 (persistent: 1.0, §R7); it moves with the blend fixes.

Tests

New test_phs_log_reference.jl (12 testitems): constant-value invariance, negative data, invalid references, callables answering deriv (declared or through kw...), allocation pins.
Pins: §F4 and §F5 now pass; new pins §F7 and §R11 (a constant answers an order-0 deriv tuple with its value) pass. New pin §F6 (a clear error when a value-only reference gets a derivative query) stays broken, as do §R2, §R7, §R9 and §R10.
PHS test files on 1.13 and LTS: 416 pass / 31 broken (open pins). Lints and Aqua green.

Replace the exported `ConstantRef` / `PHSLogTransform` and the keywords `reference_interp` / `reference_data` with one keyword, `log_reference`: `nothing`, a nonzero constant (used as given), or a callable ρ₀(q) that is called as before, `ρ₀(q)` and `ρ₀(q; deriv = ops)`. - Validate the reference at build, at grid precision: every data/ρ₀ must be positive and finite, and the error names the node; Bool and arrays are rejected. - Drop `reference_data`: its only use, a PHS of ρ₀ on the data grid, gives the constant-reference result. - One-shot methods forward their keywords to the persistent constructor; the one-shot blend_factor default stays 2.0. - Move the log-reference code into src/phs/phs_log_reference.jl. - Docs: fix the log-transform derivative formulas and the NaN example. - Tests: new test_phs_log_reference.jl; pins F4, F5, F7 and R11 pass, F6 is added as broken.

github-actions · 2026-10-05T19:30:49Z

FastInterpolations.jl Benchmarks

All benchmarks (71 total, click to expand)

Benchmark	Current: `b7b2329`	Previous	Imm. Ratio	Grad. Ratio
`10_nd_construct/bicubic_2d`	`35477` ns	`36669.0` ns	`0.967`	`0.934`
`10_nd_construct/bilinear_2d`	`587.1` ns	`607.5` ns	`0.966`	`0.971`
`10_nd_construct/phs_2d`	`1067910.7` ns	`1111486.5` ns	`0.961`	`-`
`10_nd_construct/phs_3d`	`17539243.2` ns	`17814900.4` ns	`0.985`	`-`
`10_nd_construct/tricubic_3d`	`314572` ns	`334816.0` ns	`0.94`	`0.877`
`10_nd_construct/trilinear_3d`	`1775.14` ns	`1867.9` ns	`0.95`	`1.016`
`11_nd_eval/bicubic_2d_batch`	`1421.7` ns	`1423.6` ns	`0.999`	`1.02`
`11_nd_eval/bicubic_2d_scalar`	`15.73` ns	`15.8` ns	`0.994`	`0.881`
`11_nd_eval/bilinear_2d_scalar`	`8.51` ns	`8.5` ns	`1.0`	`0.991`
`11_nd_eval/phs_2d_batch`	`38835` ns	`38495.1` ns	`1.009`	`-`
`11_nd_eval/phs_2d_scalar`	`305.36` ns	`300.7` ns	`1.015`	`-`
`11_nd_eval/phs_3d_batch`	`234695` ns	`238726.8` ns	`0.983`	`-`
`11_nd_eval/phs_3d_scalar`	`2227.38` ns	`2263.6` ns	`0.984`	`-`
`11_nd_eval/tricubic_3d_batch`	`3049.7` ns	`3072.8` ns	`0.992`	`0.934`
`11_nd_eval/tricubic_3d_scalar`	`34.46` ns	`34.1` ns	`1.012`	`1.012`
`11_nd_eval/trilinear_3d_scalar`	`12.82` ns	`13.5` ns	`0.948`	`0.921`
`12_cubic_eval_gridquery/range_random`	`3015.26` ns	`2876.4` ns	`1.048`	`0.713`
`12_cubic_eval_gridquery/range_sorted`	`2848.36` ns	`2871.8` ns	`0.992`	`0.675`
`12_cubic_eval_gridquery/vec_random`	`8921.96` ns	`8285.7` ns	`1.077`	`0.975`
`12_cubic_eval_gridquery/vec_sorted`	`2377.88` ns	`2606.3` ns	`0.912`	`0.738`
`13_nd_oneshot_gridquery/bicubic_2d_rand_rand`	`60745.2` ns	`60304.8` ns	`1.007`	`0.933`
`13_nd_oneshot_gridquery/bicubic_2d_sort_rand`	`56788.7` ns	`56775.2` ns	`1.0`	`0.916`
`13_nd_oneshot_gridquery/bicubic_2d_sort_sort`	`55062.4` ns	`55050.0` ns	`1.0`	`0.914`
`13_nd_oneshot_gridquery/bilinear_2d_rand_rand`	`16785.94` ns	`16483.6` ns	`1.018`	`1.021`
`13_nd_oneshot_gridquery/bilinear_2d_sort_rand`	`10057.1` ns	`8824.3` ns	`1.14`	`1.079`
`13_nd_oneshot_gridquery/bilinear_2d_sort_sort`	`5622.16` ns	`5539.6` ns	`1.015`	`1.099`
`14_series_oneshot_batch/constant_inplace_vec_k8_q1000_rand`	`14536.4` ns	`13582.5` ns	`1.07`	`0.993`
`14_series_oneshot_batch/linear_inplace_vec_k8_q1000_rand`	`13845.1` ns	`13843.9` ns	`1.0`	`0.962`
`15_gridded_query/cubic_oneshot_2d_40x32`	`41398.7` ns	`42532.6` ns	`0.973`	`0.934`
`15_gridded_query/cubic_persistent_2d_40x32`	`8315.6` ns	`8328.6` ns	`0.998`	`0.976`
`15_gridded_query/linear_oneshot_2d_40x32`	`1534.48` ns	`1519.6` ns	`1.01`	`0.995`
`15_gridded_query/linear_oneshot_3d_12x10x8`	`1921.6` ns	`1922.2` ns	`1.0`	`0.954`
`15_gridded_query/linear_persistent_2d_40x32`	`1505.22` ns	`1497.4` ns	`1.005`	`1.0`
`15_gridded_query/linear_persistent_3d_12x10x8`	`1893.14` ns	`1874.7` ns	`1.01`	`0.951`
`16_phs/construct_g0100`	`35210.56` ns	`36642.8` ns	`0.961`	`-`
`16_phs/construct_g1000`	`35142` ns	`36293.9` ns	`0.968`	`-`
`16_phs/eval_q00001`	`111` ns	`111.3` ns	`0.997`	`-`
`16_phs/eval_q00100`	`11921.2` ns	`11955.4` ns	`0.997`	`-`
`16_phs/eval_q10000`	`1173461.9` ns	`1175100.3` ns	`0.999`	`-`
`16_phs/oneshot_q00001`	`36353.52` ns	`37900.1` ns	`0.959`	`-`
`16_phs/oneshot_q10000`	`1783831.6` ns	`1782651.5` ns	`1.001`	`-`
`1_cubic_oneshot/q00001`	`454.86` ns	`454.4` ns	`1.001`	`0.999`
`1_cubic_oneshot/q10000`	`29572.5` ns	`29592.4` ns	`0.999`	`0.682`
`2_cubic_construct/g0100`	`1302.66` ns	`1297.8` ns	`1.004`	`0.994`
`2_cubic_construct/g1000`	`12581.6` ns	`13302.9` ns	`0.946`	`0.998`
`3_cubic_eval/q00001`	`9.52` ns	`9.6` ns	`0.991`	`0.465`
`3_cubic_eval/q00100`	`302.56` ns	`305.4` ns	`0.991`	`0.684`
`3_cubic_eval/q10000`	`28833.1` ns	`29054.4` ns	`0.992`	`0.676`
`4_linear_oneshot/q00001`	`26.75` ns	`26.7` ns	`1.0`	`1.011`
`4_linear_oneshot/q10000`	`18599.9` ns	`18576.7` ns	`1.001`	`1.009`
`5_linear_construct/g0100`	`39.67` ns	`36.9` ns	`1.076`	`1.08`
`5_linear_construct/g1000`	`282.73` ns	`288.0` ns	`0.982`	`1.035`
`6_linear_eval/q00001`	`8.91` ns	`8.7` ns	`1.023`	`0.846`
`6_linear_eval/q00100`	`200.16` ns	`200.0` ns	`1.001`	`1.02`
`6_linear_eval/q10000`	`18383.5` ns	`18379.4` ns	`1.0`	`1.01`
`7_cubic_range/scalar_query`	`5.61` ns	`5.7` ns	`0.984`	`0.697`
`7_cubic_vec/scalar_query`	`9.71` ns	`8.5` ns	`1.141`	`0.882`
`8_cubic_multi/construct_s001_q100`	`508.96` ns	`509.1` ns	`1.0`	`0.995`
`8_cubic_multi/construct_s010_q100`	`4244.18` ns	`4515.9` ns	`0.94`	`0.99`
`8_cubic_multi/construct_s100_q100`	`39576.2` ns	`41612.9` ns	`0.951`	`1.0`
`8_cubic_multi/eval_s001_q100`	`452.44` ns	`429.6` ns	`1.053`	`0.776`
`8_cubic_multi/eval_s010_q100`	`1315.68` ns	`1319.3` ns	`0.997`	`0.9`
`8_cubic_multi/eval_s010_q100_scalar_loop`	`2239.2` ns	`2185.7` ns	`1.024`	`0.965`
`8_cubic_multi/eval_s100_q100`	`10003.8` ns	`10490.6` ns	`0.954`	`0.981`
`8_cubic_multi/eval_s100_q100_scalar_loop`	`3275.2` ns	`3365.3` ns	`0.973`	`0.964`
`9_nd_oneshot/bicubic_2d`	`34622.1` ns	`34441.5` ns	`1.005`	`0.924`
`9_nd_oneshot/bilinear_2d`	`456.86` ns	`451.0` ns	`1.013`	`1.01`
`9_nd_oneshot/phs_2d`	`1323302.7` ns	`1359800.9` ns	`0.973`	`-`
`9_nd_oneshot/phs_3d`	`20037835.9` ns	`20277302.2` ns	`0.988`	`-`
`9_nd_oneshot/tricubic_3d`	`321116.2` ns	`322555.7` ns	`0.996`	`0.863`
`9_nd_oneshot/trilinear_3d`	`832.6` ns	`844.6` ns	`0.986`	`0.908`

⚠️ Performance Regression Confirmed ⚠️

After re-running 3 flagged benchmark(s) 10 time(s), 2 regression(s) confirmed.

Benchmark	Current	Previous	Imm. Ratio	Grad. Ratio	Tier
`13_nd_oneshot_gridquery/bilinear_2d_sort_rand`	`10057.1` ns	`8824.3` ns	`1.14`	`1.079`	immediate
`7_cubic_vec/scalar_query`	`9.71` ns	`8.5` ns	`1.141`	`0.882`	immediate

Thresholds: immediate > 1.1x (vs latest master), gradual > 1.1x (vs sliding window)

Runner: znver3|4c — AMD EPYC 7763 64-Core Processor, julia 1.13.1. Times are min-merged and compared only against this same machine's history.

This comment was automatically generated by Benchmark workflow.

@allocated

A single warm-up only runs the stencil-coefficient cache-miss path. On Julia LTS the first cache-hit run then compiles inside @allocated (~192 KB) when the worker is fresh, as under the parallel runner. The same happens on master; this is a test warm-up gap, not a regression.

Copilot

Copilot review overview

🟢 Approval recommended

The API migration is consistent across implementation, tests, documentation, and examples, with no unresolved correctness issues found.

Review effort: Balanced
Findings: None

What changed in this PR

Refactors the PHS log transform into a single validated log_reference API.

Changes:

Replaces legacy reference types and keywords with log_reference.
Adds validation, constant/callable reference handling, and derivative support.
Updates tests, documentation, and example scripts.

File	Description
`src/FastInterpolations.jl`	Removes legacy exports.
`src/phs/phs.jl`	Loads reference implementation.
`src/phs/phs_types.jl`	Removes public reference types.
`src/phs/phs_log_reference.jl`	Implements reference resolution and validation.
`src/phs/phs_interpolant.jl`	Adds the new constructor keyword.
`src/phs/phs_eval.jl`	Evaluates constant and callable references.
`src/phs/phs_oneshot.jl`	Forwards constructor keywords.
`test/test_phs_nd.jl`	Migrates existing tests.
`test/test_phs_log_reference.jl`	Adds comprehensive reference tests.
`test/test_phs_broken_pins.jl`	Updates and promotes regression pins.
`docs/src/interpolation/phs.md`	Documents behavior and formulas.
`docs/src/api/phs.md`	Updates API reference.
`scripts/phs/phs_density_comparison.jl`	Migrates comparison script.
`scripts/phs/phs_density_comparison_simplified.jl`	Migrates simplified script.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

mgyoo86 · 2026-10-05T21:06:37Z

@twilsonco
This is the first follow-up to #136.
It only reshapes the public API of the log transform.

One keyword, log_reference, now replaces ConstantRef, PHSLogTransform, reference_interp and reference_data.
It takes a nonzero number or any callable, such as an interpolant. For the same reference, results are bit-identical to before; the before/after mapping is in the PR description.

As far as I can tell, every case the old API covered can still be expressed. If something in your use case no longer works or is harder to write now, please let me know here.

Performance and the other open items will come in the next follow-up PRs.
Thanks!

codecov · 2026-10-06T00:04:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.29%. Comparing base (f69b7af) to head (b7b2329).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #209   +/-   ##
=======================================
  Coverage   96.28%   96.29%           
=======================================
  Files         171      172    +1     
  Lines       15467    15484   +17     
=======================================
+ Hits        14893    14910   +17     
  Misses        574      574

Files with missing lines	Coverage Δ
src/FastInterpolations.jl	`100.00% <ø> (ø)`
src/phs/phs_eval.jl	`92.03% <100.00%> (-0.05%)`	⬇️
src/phs/phs_interpolant.jl	`100.00% <100.00%> (ø)`
src/phs/phs_log_reference.jl	`100.00% <100.00%> (ø)`
src/phs/phs_oneshot.jl	`100.00% <100.00%> (ø)`
src/phs/phs_types.jl	`100.00% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

twilsonco · 2026-10-06T06:05:15Z

Thanks @mgyoo86. I'll review how I'm using it and update here to confirm all looks good.

mgyoo86 · 2026-10-13T21:01:07Z

Hi @twilsonco,

I merged this PR to prepare for a new release, as several other fixes and performance improvements are already lined up. I think the new API should be fine, but please feel free to comment here or open an issue if you run into any problems.

mgyoo86 requested a balanced review from Copilot October 5, 2026 20:21

Copilot started reviewing on behalf of mgyoo86 October 5, 2026 20:32

View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

mgyoo86 added 3 commits October 6, 2026 14:19

Merge branch 'master' into refac/phs-log-reference

cee93a6

Merge branch 'master' into refac/phs-log-reference

df50a9c

Merge branch 'master' into refac/phs-log-reference

df88f47

mgyoo86 mentioned this pull request Oct 7, 2026

test(search): compare cubic values across search policies with a ULP budget #212

Merged

mgyoo86 added 2 commits October 7, 2026 16:30

Merge branch 'master' into refac/phs-log-reference

05597f9

Merge branch 'master' into refac/phs-log-reference

b7b2329

mgyoo86 merged commit 28b05fe into master Oct 13, 2026
24 of 25 checks passed

mgyoo86 deleted the refac/phs-log-reference branch October 13, 2026 21:42

mgyoo86 mentioned this pull request Oct 13, 2026

fix(phs): ship PHS as experimental and reject inputs it cannot evaluate #222

Merged

mgyoo86 referenced this pull request Oct 14, 2026

chore: update version to 0.4.20 and benchmark results in README

608074d

Navigation Menu

Repository navigation

refactor(phs): one `log_reference` keyword for the log transform - #209

refactor(phs): one `log_reference` keyword for the log transform#209
mgyoo86 merged 7 commits into
masterfrom
refac/phs-log-reference

mgyoo86 commented Oct 5, 2026

Uh oh!

github-actions Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

mgyoo86 commented Oct 5, 2026

Uh oh!

codecov Bot commented Oct 6, 2026 •

edited

Loading

Uh oh!

twilsonco commented Oct 6, 2026

Uh oh!

Uh oh!

mgyoo86 commented Oct 13, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mgyoo86 commented Oct 5, 2026

Summary

Behavior changes

Unchanged

Tests

Uh oh!

github-actions Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

FastInterpolations.jl Benchmarks

⚠️ Performance Regression Confirmed ⚠️

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

mgyoo86 commented Oct 5, 2026

Uh oh!

codecov Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

twilsonco commented Oct 6, 2026

Uh oh!

Uh oh!

mgyoo86 commented Oct 13, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 5, 2026 •

edited

Loading

codecov Bot commented Oct 6, 2026 •

edited

Loading