Skip to content

fix(transforms): avoid NaN at CurveInterpolator's lowest sample point#1005

Merged
mairas merged 1 commit into
mainfrom
fix/curveinterpolator-nan-lowest-sample
Jun 16, 2026
Merged

fix(transforms): avoid NaN at CurveInterpolator's lowest sample point#1005
mairas merged 1 commit into
mainfrom
fix/curveinterpolator-nan-lowest-sample

Conversation

@mairas

@mairas mairas commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Bug

CurveInterpolator::set(x) returns NaN when x equals the smallest sample's input. The interval search starts at begin() and only advances while input > it->input_, so at the lowest point it never advances: x0 and x1 both reference that same sample, and the interpolation (…)/(x1 - x0) divides by zero.

Fix

Guard the zero-width interval: when x1 == x0, output the sample's y directly.

How it was found

Surfaced by executing the test suite on a HALMET device (test_curve_interpolator_exact_sample_points — "Expected 0 Was nan"). CI only compiles the Unity suites (pio test --without-testing), so it never caught this. The existing test in test_transforms covers the fix; it passes on-device after this change.

set() at an input equal to the smallest sample's x left the search at
begin(), so x0 and x1 were the same point and the interval divide was
0/0 -> NaN. Return the sample's y for a zero-width interval.
@mairas

mairas commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Automated review

Verdict: Correct, minimal fix. Approve. Root cause and fix confirmed by source-tracing; no blocking issues.

Root cause — confirmed

samples_ is a std::set<Sample> ordered by input_ alone. In set(), after the input < x0 early return, the search loop (lines 66–74) advances it only while input > it->input_. At the lowest sample point input == begin()->input_, so the loop never advances: it stays at begin(), x1 = it->input_ equals x0 = begin()->input_, and the original (…)/(x1 - x0) was 0/0 → NaN. Confirmed.

Fix correctness — confirmed for all three paths

  • Lowest sample point: x1 == x0, y1 == y0 (same sample); fix outputs y1 = that sample's y. Correct (returns the sample's own y, not a hardcoded 0).
  • Interior exact hit (e.g. query x=2 on samples 1,2,3): loop consumes x=1 into x0, breaks at x=2, so x1 != x0 and the normal path yields y1. Unchanged by the fix.
  • Between points: x1 != x0, normal path, unchanged.

x1 == x0 is reachable only at begin() — any later exact hit has already advanced x0 to the previous sample.

Degenerate paths — checked

  • Duplicate sample x: Cannot occur. The set's ordering key is input_ only, so std::set treats equal-x samples as equivalent and rejects the second insert. No mid-table x1 == x0. The y1 choice is moot for duplicates and correct at begin().
  • Float ==: Appropriate here. In the trigger case x0 and x1 are both copied from the same stored it->input_ (the iterator never advanced), so they are bit-identical; == is exact and no epsilon is warranted. The guard also doubles as a general divide-by-zero guard for any bit-identical pair.

Style / conventions

Minimal, idiomatic, and the comment explains the why (zero-width interval at begin()) rather than narrating the change — matches AGENTS.md comment guidance. No nits.

Non-blocking note

test_curve_interpolator_exact_sample_points asserts the lowest point returns 0.0, but that sample's y is itself 0 — so the test does not distinguish "returns the sample's y" from "returns 0". The fix is correct by source-tracing regardless; if you want the test to pin the behavior, a fixture whose lowest sample has a nonzero y (e.g. (0, 5)) would make the output_ = y1 choice explicit. Optional.

@mairas mairas merged commit d444fb8 into main Jun 16, 2026
24 checks passed
@mairas mairas deleted the fix/curveinterpolator-nan-lowest-sample branch June 16, 2026 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant