Repository navigation
Conversation
Fixes #857. The previous behaviour remains available by setting the `nansafe_mode` preference to `false`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Check your benchmarks with #859 perhaps |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #858 +/- ##
=======================================
Coverage 91.23% 91.23%
=======================================
Files 11 11
Lines 1072 1072
=======================================
Hits 978 978
Misses 94 94 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I don't think we should merge this until something like #859 is in. I didn't fully get the objection to that PR, it was a very long response but feel free to modify as you see fit. |
|
I benchmarked #859 on more examples and studied a few other alternatives, but none of them was able to consistently outperform this PR here. Regarding #859 specifically, I think master/this PR is better since
|
|
As I mentioned in #857, my strong opinion is that the correct mode should be the default, regardless of its performance impact. Unsafe performance optimization must be an opt-in, similar to Any user can still opt into the faster but unsafe version by setting the preference. |
This is obviously false. You can test by putting a |
Who cares about "conceptually", though? The point is to get the result and to get it quickly.
This is from benchmarking a completely useless |
|
I disagree with both points. There's one thing we want to achieve: zero partials out for zero partials in. The approach on master clearly enforces this, and it's easy to check and validate for any reader. The approach in #859 is indirect and does not work in general, hence Moreover, on my computer I just couldn't see a consistent advantage of #859 in the benchmarks. Neither in microbenchmarks nor in full derivative/gradient calls. Based on my experiments, I just can't reproduce that it is clearly superior. On the other hand, what my experiments and study of some alternatives revealed was that the compiler much more consistently and reliably vectorizes the approach on master whereas the approach on #859 is compiled in a much less predictable way, even in the alternatives I tried. This seemed to explain also some of the performance regressions of #859. All in all, in my experiments #859 doesn't seem to be a clear winner but come with a few caveats and disadvantages. |
Fixes #857.
This PR enables NaN-safe mode by default. The previous behaviour can still be selected by setting the
nansafe_modepreference tofalse. The documentation section is rewritten accordingly, and the outdated "~5%-10%" performance claim is replaced with the numbers below. Since the CI matrix already sets the preference explicitly totrueandfalse, both modes remain tested.With this PR, by default:
(previously
[Inf, NaN]andDual{:tag}(-Inf,NaN)).I bumped the version to 1.5.0 since the default behaviour changes. Happy to revert that if you'd prefer a patch release.
Timings
Median times (Chairmarks
@be,seconds=3). Each mode ran in a fresh process, and the runs alternated twice (false,true,false,true). Both runs agree within a few percent, so the table shows the second run.gradient(rosenbrock, rand(1000))hessian(rosenbrock, rand(10))gradient(x -> sum(exp, x), rand(100))jacobian(cumprod, rand(10))derivative(x -> sin(x) * exp(x) / x, 1.3)Overhead shows up in code that does little more than multiply and add (Rosenbrock), where the compare and select are a sizeable fraction of the work. It is within noise once transcendental functions or memory traffic dominate.
Benchmark script
versioninfo()Generated code
In NaN-safe mode, each 2-wide vector of partials gets one extra compare and one extra mask instruction (
fcmeq+bicon aarch64,fcmp une+selectin LLVM IR). The code remains branch-free and fully vectorized.@code_native debuginfo=:none ForwardDiff._mul_partials(p, p, 2.0, 1.0)withp::Partials{4,Float64}NaN-safe mode disabled:
NaN-safe mode enabled:
@code_llvm debuginfo=:none x * xwithx::Dual{Nothing,Float64,4}(function body)NaN-safe mode disabled:
NaN-safe mode enabled:
@code_llvm debuginfo=:none exp(x)withx::Dual{Nothing,Float64,4}(function body)NaN-safe mode disabled:
NaN-safe mode enabled:
Tests
Pkg.test()passes locally in both modes: 9723 tests with the new default and 9641 withnansafe_mode = false.🤖 Generated with Claude Code