[RF] Test RooFit Hessians with Clad - #21622
Draft
guitargeek wants to merge 8 commits into
Draft
Conversation
Test Results 20 files 20 suites 2d 22h 1m 22s ⏱️ For more details on these failures, see this check. Results for commit 994beed. ♻️ This comment has been updated with latest results. |
guitargeek
force-pushed
the
roofit_clad_hessians
branch
from
March 17, 2026 09:52
e54bf52 to
2a2b0cc
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
2 times, most recently
from
March 29, 2026 18:19
6368d0c to
11eab4c
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
2 times, most recently
from
April 26, 2026 13:45
92c3589 to
a79ccd5
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
from
June 17, 2026 06:56
a79ccd5 to
db537ff
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
from
July 5, 2026 18:38
db537ff to
35d56ad
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
7 times, most recently
from
August 13, 2026 20:48
c63fa60 to
5e627d1
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
from
August 14, 2026 16:01
5e627d1 to
56addac
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
2 times, most recently
from
August 17, 2026 16:41
d4726b8 to
29e1d3e
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
from
August 19, 2026 09:34
29e1d3e to
68ac8a3
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
3 times, most recently
from
August 23, 2026 19:33
c340118 to
7449980
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
3 times, most recently
from
September 3, 2026 14:17
add52a0 to
994beed
Compare
guitargeek
force-pushed
the
roofit_clad_hessians
branch
from
September 7, 2026 08:18
994beed to
9201d97
Compare
This was referenced Sep 7, 2026
vgvassilev
pushed a commit
to guitargeek/clad
that referenced
this pull request
Sep 7, 2026
Two instantiations of one template ask for the same derivative name, so
the lookup for the second finds the derivative built for the first:
template <typename T> double scale(double x, T n) { return x * x * n; }
double sumOfScales(double x) { return scale(x, 2.) + scale(x, 3); }
clad::hessian builds scale_pushforward_pullback for T = double and injects
it into the enclosing namespace. The T = int call finds only that overload.
With one candidate Sema builds a plain DeclRefExpr instead of an overload
set, and noOverloadExists compared only the argument count, so the call was
built anyway:
error: cannot initialize a parameter of type 'double *' with an rvalue
of type 'int *'
note: passing argument to parameter '_d_n' here
<cmath> reaches the same state, where a floating-point and an integral
exponent are two instantiations of pow_pushforward. That broke the Hessian
of every RooFit likelihood containing a RooBernstein, whose integral mixes
std::pow(x, j + 1.) with std::pow(-1., j - i) (root-project/root#21622).
Run the single candidate through overload resolution and report no overload
when the arguments are not viable, so the caller derives the one that fits.
guitargeek
added a commit
to guitargeek/clad
that referenced
this pull request
Sep 7, 2026
Two instantiations of one template ask for the same derivative name, so
the lookup for the second finds the derivative built for the first:
template <typename T> double scale(double x, T n) { return x * x * n; }
double sumOfScales(double x) { return scale(x, 2.) + scale(x, 3); }
clad::hessian builds scale_pushforward_pullback for T = double and injects
it into the enclosing namespace. The T = int call finds only that overload.
With one candidate Sema builds a plain DeclRefExpr instead of an overload
set, and noOverloadExists compared only the argument count, so the call was
built anyway:
error: cannot initialize a parameter of type 'double *' with an rvalue
of type 'int *'
note: passing argument to parameter '_d_n' here
<cmath> reaches the same state, where a floating-point and an integral
exponent are two instantiations of pow_pushforward. That broke the Hessian
of every RooFit likelihood containing a RooBernstein, whose integral mixes
std::pow(x, j + 1.) with std::pow(-1., j - i) (root-project/root#21622).
Run the single candidate through overload resolution and report no overload
when the arguments are not viable, so the caller derives the one that fits.
guitargeek
added a commit
to vgvassilev/clad
that referenced
this pull request
Sep 7, 2026
Two instantiations of one template ask for the same derivative name, so
the lookup for the second finds the derivative built for the first:
template <typename T> double scale(double x, T n) { return x * x * n; }
double sumOfScales(double x) { return scale(x, 2.) + scale(x, 3); }
clad::hessian builds scale_pushforward_pullback for T = double and injects
it into the enclosing namespace. The T = int call finds only that overload.
With one candidate Sema builds a plain DeclRefExpr instead of an overload
set, and noOverloadExists compared only the argument count, so the call was
built anyway:
error: cannot initialize a parameter of type 'double *' with an rvalue
of type 'int *'
note: passing argument to parameter '_d_n' here
<cmath> reaches the same state, where a floating-point and an integral
exponent are two instantiations of pow_pushforward. That broke the Hessian
of every RooFit likelihood containing a RooBernstein, whose integral mixes
std::pow(x, j + 1.) with std::pow(-1., j - i) (root-project/root#21622).
Run the single candidate through overload resolution and report no overload
when the arguments are not viable, so the caller derives the one that fits.
The code generated for a RooLognormal with useStandardParametrization() called
RooFit::Detail::MathFuncs::logNormalEvaluateStandard, which does not exist --
the function is called logNormalStandard. Any codegen or AD fit of such a pdf
failed to compile with "no member named 'logNormalEvaluateStandard'".
This went unnoticed because the LognormalStandard case in testRooFuncWrapper
builds its pdf with
Lognormal::model(x[...], mu[...], k[...], true)
and the factory quietly dropped that last argument, so the test was really a
duplicate of the Lognormal one. RooFactoryWSTool::asINT(), which is also the
conversion used for bool constructor arguments, is atoi(), and atoi("true") is
zero. Teach it about the spelled-out literals, which affects every factory
string that writes a bool that way, and makes the existing test exercise what
it says it does.
Verified that the generated code now agrees exactly with the reference backend,
for the nominal likelihood value and over a scan of the shape parameters.
🤖 Done with the help of AI
Add second-derivative support to the function interfaces, following the existing HasGradient()/Gradient() pattern: - IBaseFunctionMultiDimTempl gets HasHessian() (default false) and bool Hessian(x, hess), filling a row-major NDim() x NDim() array and returning false when not implemented. The full-matrix layout and bool return match the Minuit2 FCNBase convention. Note that FitMethodFunction already had a bool Hessian() override filling a packed lower triangle; that pre-existing layout now deviates from the base-class contract and is left untouched here. - IBaseFunctionOneDim gets HasHessian(), SecondDerivative(), and a multi-dim-compatible Hessian(), implemented via a new private DoSecondDerivative() that throws by default. - GradFunctor takes an optional Hessian std::function in its (f, dim, gradient) constructor. - GradFunctor1D takes an optional second-derivative std::function in its two-function constructor. Its member-pointer constructor template needed an is_member_pointer constraint so that three plain function pointers select the new std::function overload instead. This lets minimizers and, in particular, the RooFit codegen backend query externally provided second derivatives, which is needed to support Clad Hessians through opaque functor calls. 🤖 Done with the help of AI
The codegen backend already emitted a custom pullback for
RooFunctorBinding-style pdfs, forwarding to the wrapped functor's
Gradient(). That covers gradients, but Clad Hessians run in
reverse-over-forward mode: the forward pass needs a
`<name>_pushforward`, and the reverse pass over that opaque call
needs a `<name>_pushforward_pullback`. Without them, Hessian
generation failed for any model containing a bound functor.
When the wrapped function reports HasHessian(), additionally emit
- roo_functor_<addr>_pushforward: value plus grad . dx, from the
functor's operator() and Gradient(), and
- roo_functor_<addr>_pushforward_pullback: the exact adjoint of the
pushforward, using the functor's Hessian(),
d_x[i] += d_y.value * grad[i] + d_y.pushforward * (H dx)[i]
d_dx[i] += d_y.pushforward * grad[i],
following the signature convention Clad uses for custom pushforward
pullbacks (see clad's test/Hessian/NestedArrays.C). The declared code
includes Clad's BuiltinDerivatives.h itself, since the functor
declaration is JIT-ed before RooFuncWrapper includes CladDerivator.h.
Functors that do not implement Hessian() keep the previous
gradient-only behavior.
🤖 Done with the help of AI
Enable the Hessian cross-check (Minuit's numeric Hesse on the reference fit vs. clad::hessian on the generated code) for every test in the suite. All Clad Hessians were validated against finite differences of the exact Clad gradient using the writeDebugMacro() output; where fit errors still differ, the numeric Hesse of the reference fit is the imprecise side. - Add a per-test hesseTolerance for the parameter-error comparison, since Minuit's numeric Hesse and the analytic Clad Hessian agree less tightly (1e-3 .. 5e-2 relative) than the fitted values do. - Implement analytic Hessians for the RooFunctor test functors, so that test exercises the new externally-bound-functor support end-to-end. - Fix degenerate test models whose Hessians were singular, making the error comparison meaningless: the Gaussian/RooFormulaVar models depended only on mu + shift (shift is now constant), and the Bernstein pdf was invariant under a common coefficient rescaling (c0 is now constant). - Improve the conditioning of ill-defined fits: RooLandau3 (sl = 10, comparable to the observable window) generates 10k events via a new nEvents argument, and the RooFunctor Gaussian starts at mu = 2, sigma = 1.5 so the data constrain all parameters and the fitted values are away from zero. - Skip constant parameters when randomizing initial values; factory constants have an infinite range, so randomization pushed them to +-inf. 🤖 Done with the help of AI
Clad computes Hessians in reverse-over-forward mode: the forward pass needs a `<name>_pushforward` for the opaque roo_outer_wrapper call, and the reverse pass over that call needs the matching `<name>_pushforward_pullback`. Without them, Hessian generation failed for any model containing a RooONNXFunc. Emit both from RooONNXFunc::initialize(), into the same clad::custom_derivatives namespace as the existing custom pullback. Both are plain C++ built on the already-emitted exact gradient pullback, so no additional Clad differentiation happens: - roo_outer_wrapper_pushforward returns the function value plus the directional derivative grad . d_input, both exact. - roo_outer_wrapper_pushforward_pullback needs second derivatives only as the Hessian-vector product H . d_input. SOFIE emits Clad pullbacks for its operators but no pushforwards, so differentiating the model code again would silently fall back to numerical differentiation of the value. Instead, evaluate the product as a central finite difference of the exact generated gradient along the normalized tangent direction. The step size (~cbrt of the float machine epsilon, scaled to the input magnitude) balances the float-precision noise of the SOFIE gradient against the truncation error; the result agrees with a float64 PyTorch reference Hessian to ~2e-4 relative accuracy. The existing test models cannot validate this: a ReLU MLP is piecewise-linear in its inputs, so its input Hessian vanishes almost everywhere. Make the activation function a model-class argument in create_onnx_model.py and add tanh variants of both models, together with a torch.autograd.functional.hessian reference saved in the RooFit parameter ordering (SOFIE generates Tanh as a plain std::tanh loop, which Clad reverse-differentiates natively). New tests: the single-tensor 10x10 Hessian, the two-tensor 15x15 Hessian including the cross-tensor blocks, and the square of a RooONNXFunc via RooProduct, whose H(f^2) = 2 (f H + grad grad^T) exercises the primal-value adjoint term that stays dormant when the ONNX function is the top-level function. 🤖 Done with the help of AI
Now that the SOFIE-generated inference code supports Clad forward-mode differentiation, replace the finite-difference-of-gradient approximation for RooONNXFunc second derivatives with exact Hessian-vector products. RooONNXFunc::initialize() now additionally requests a forward-mode derivative of the inner wrapper, keeps a persistent zero-weight tangent Session (via the generated SetWeightsToZero()), declares a directional-derivative wrapper around the generated pushforward, and reverse-differentiates it with respect to both the inputs and the tangent direction. The resulting pullback yields the exact Hessian-vector product and the gradient in a single reverse pass, so the emitted pushforward_pullback is both exact and cheaper than the previous three-gradient finite-difference stencil. The emitted second-order pullback constructs fresh adjoint Session objects on every call, working around a Clad bug where generated second-order pullbacks do not restore intermediate adjoint state. Differentiating with respect to the tangent direction as well works around a Clad custom-derivative lookup limitation with non-varied pointer arguments (and its adjoint is the gradient, which is needed anyway). Measured accuracy against the float64 PyTorch reference Hessian improves from 2.3e-6 (finite differences) to 1.9e-9, so the Hessian tolerances in testRooONNXFunc are tightened from 1e-3 to 1e-5, matching the gradient checks. 🤖 Done with the help of AI
guitargeek
force-pushed
the
roofit_clad_hessians
branch
from
September 7, 2026 11:57
9201d97 to
14475ea
Compare
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
1 participant
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.
No description provided.