Skip to content

[Tests] Pass abs=0 to pytest.approx in f64 adstack gradient asserts s…

b502ac8
Select commit
Loading
Failed to load commit list.
Open

[AutoDiff] Autodiff 6: Adstack regression tests #491

[Tests] Pass abs=0 to pytest.approx in f64 adstack gradient asserts s…
b502ac8
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Apr 21, 2026 in 26m 12s

Code review found 1 potential issue

Found 1 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit tests/python/test_adstack.py:391-397 Inaccurate comment: abs=1e-12 floor does not cause missed f32-narrowing regressions

Annotations

Check warning on line 397 in tests/python/test_adstack.py

See this annotation in the file changed.

@claude claude / Claude Code Review

Inaccurate comment: abs=1e-12 floor does not cause missed f32-narrowing regressions

The comments in `_run_basic_gradient` and `test_adstack_basic_gradient_f64` incorrectly claim that a 1e-12 abs floor "would miss an f32-narrowing regression" — in reality, f32 vs f64 differences for `0.95**n_iter` are ~1e-7 to 1e-8 (4-5 orders of magnitude *larger* than 1e-12), so the floor would still catch f32-narrowing regressions. The correct justification for `abs_tol=0` is that without it the effective tolerance becomes ~1e-12 absolute (dominating the 1e-14 relative), which misses sub-f32-