[hist] Allow TF1 lambdas to receive ndim and npar as arguments - Issue #22071#22509
[hist] Allow TF1 lambdas to receive ndim and npar as arguments - Issue #22071#22509medeirosdev wants to merge 5 commits into
Conversation
Users constructing TF1 with a lambda had no safe way to do bound
checking on variables and parameters arrays, since the current API
only exposes raw pointers with no size information.
This adds support for a 4-argument lambda signature:
[](const double *x, std::size_t ndim,
const double *p, std::size_t npar) -> double
TF1Builder now detects this signature at compile time via
std::is_invocable_r_v and wraps it in an adapter that passes
fNdim and fNpar captured from the TF1 object. The existing
2-argument API is fully preserved.
Closes root-project#22071
|
Hi, thanks for the PR. Would it be possible to have |
Yes! nice one Co-authored-by: Yanzhao Wang (王彦昭) <yannzhaow@gmail.com>
nice suggestions Co-authored-by: Yanzhao Wang (王彦昭) <yannzhaow@gmail.com>
Both branches set fType to kTemplScalar, so move it before the if constexpr to avoid the duplication. Also fix extra parenthesis from applied suggestion and sync the Func* specialization to use init-captures and make_unique consistently.
std::span would definitely be a cleaner API, but t challenge is that it's C++20, while ROOT currently targets C++17 as the minimum. one option would be to support it as an additional signature, guarded by #ifdef __cpp_lib_span, so users on C++20 can use span while C++17 users keep the size_t version. Would that be the right direction, or would you prefer to keep this PR focused on the size_t approach and handle span in a follow-up? |
|
Hi, Yes. If I'm not wrong, ifdef guards are pretty common in ROOT database. So it would be really nice to also have std::span under a ifdef guard in this PR. |
When compiled with C++20 (guarded by #ifdef __cpp_lib_span), TF1 now also accepts lambdas with the cleaner span-based signature: [](std::span<const double> x, std::span<const double> p) -> double This is detected via if constexpr before the size_t signature, so both are supported and C++17 users are unaffected. The include of <span> is guarded by __has_include to avoid breaking older compilers. Adds a matching test in LambdaWithSpan (compiled only under C++20).
|
Done! Added std::span support guarded by #ifdef __cpp_lib_span, so it's only active on C++20. The detection happens via if constexpr before the size_t path, so C++17 users are unaffected. Also added a test for the span signature. |
Users constructing TF1 with a lambda had no safe way to do bound checking on variables and parameters arrays, since the current API only exposes raw pointers with no size information.
This adds support for a 4-argument lambda signature:
[](const double *x, std::size_t ndim,
const double *p, std::size_t npar) -> double
TF1Builder now detects this signature at compile time via std::is_invocable_r_v and wraps it in an adapter that passes fNdim and fNpar captured from the TF1 object. The existing 2-argument API is fully preserved.
Closes #22071
This Pull request:
Changes or fixes:
Checklist:
This PR fixes #