Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,13 @@ PyObject* VectorData(PyObject* self, PyObject*)
}


// This function implements __array__, added to std::vector python proxies and causes
// a bug (see explanation at Utility::AddToClass(pyclass, "__array__"...) in CPyCppyy::Pythonize)
// The recursive nature of this function, passes each subarray (pydata) to the next call and only
// the final buffer is cast to a lowlevel view and resized (in VectorData), resulting in only the
// first 1D array to be returned. See https://github.com/root-project/root/issues/17729
// It is temporarily removed to prevent errors due to -Wunused-function, since it is no longer added.
#if 0
//---------------------------------------------------------------------------
PyObject* VectorArray(PyObject* self, PyObject* args, PyObject* kwargs)
{
Expand All @@ -537,7 +544,7 @@ PyObject* VectorArray(PyObject* self, PyObject* args, PyObject* kwargs)
Py_DECREF(pydata);
return newarr;
}

#endif

//-----------------------------------------------------------------------------
static PyObject* vector_iter(PyObject* v) {
Expand Down Expand Up @@ -1810,8 +1817,15 @@ bool CPyCppyy::Pythonize(PyObject* pyclass, const std::string& name)
Utility::AddToClass(pyclass, "__real_data", "data");
Utility::AddToClass(pyclass, "data", (PyCFunction)VectorData);

// The addition of the __array__ utility to std::vector Python proxies causes a
// bug where the resulting array is a single dimension, causing loss of data when
// converting to numpy arrays, for >1dim vectors. Since this C++ pythonization
// was added with the upgrade in 6.32, and is only defined and used recursively,
// the safe option is to disable this function and no longer add it.
#if 0
// numpy array conversion
Utility::AddToClass(pyclass, "__array__", (PyCFunction)VectorArray, METH_VARARGS | METH_KEYWORDS /* unused */);
#endif

// checked getitem
if (HasAttrDirect(pyclass, PyStrings::gLen)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
From 82295e09c77ae61e2fd8356be792d61addf2c801 Mon Sep 17 00:00:00 2001
From: Aaron Jomy <aaronjomyjoseph@gmail.com>
Date: Mon, 31 Mar 2025 14:27:32 +0200
Subject: [PATCH] [CPyCppyy] Drop `__array__` from std::vector pythonizations

The addition of the __array__ utility to std::vector Python proxies causes a
bug where the resulting array is a single dimension, causing loss of data when
converting to numpy arrays, for >1dim vectors. The recursive nature of this
function, passes each subarray (pydata) to the next call and only the final
buffer is cast to a lowlevel view and resized (in VectorData), resulting in
only the first 1D array to be returned. See https://github.com/root-project/root/issues/17729

Since this C++ pythonization was added with the upgrade in 6.32, and is only
defined and used recursively, the safe option is to disable this function and
no longer add it. It is temporarily removed to prevent errors due to -Wunused-function
---
bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
index 9b87905bab..0510c1c6ac 100644
--- a/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
+++ b/bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
@@ -527,6 +527,13 @@ PyObject* VectorData(PyObject* self, PyObject*)
}


+// This function implements __array__, added to std::vector python proxies and causes
+// a bug (see explanation at Utility::AddToClass(pyclass, "__array__"...) in CPyCppyy::Pythonize)
+// The recursive nature of this function, passes each subarray (pydata) to the next call and only
+// the final buffer is cast to a lowlevel view and resized (in VectorData), resulting in only the
+// first 1D array to be returned. See https://github.com/root-project/root/issues/17729
+// It is temporarily removed to prevent errors due to -Wunused-function, since it is no longer added.
+#if 0
//---------------------------------------------------------------------------
PyObject* VectorArray(PyObject* self, PyObject* args, PyObject* kwargs)
{
@@ -537,7 +544,7 @@ PyObject* VectorArray(PyObject* self, PyObject* args, PyObject* kwargs)
Py_DECREF(pydata);
return newarr;
}
-
+#endif

//-----------------------------------------------------------------------------
static PyObject* vector_iter(PyObject* v) {
@@ -1810,8 +1817,15 @@ bool CPyCppyy::Pythonize(PyObject* pyclass, const std::string& name)
Utility::AddToClass(pyclass, "__real_data", "data");
Utility::AddToClass(pyclass, "data", (PyCFunction)VectorData);

+ // The addition of the __array__ utility to std::vector Python proxies causes a
+ // bug where the resulting array is a single dimension, causing loss of data when
+ // converting to numpy arrays, for >1dim vectors. Since this C++ pythonization
+ // was added with the upgrade in 6.32, and is only defined and used recursively,
+ // the safe option is to disable this function and no longer add it.
+#if 0
// numpy array conversion
Utility::AddToClass(pyclass, "__array__", (PyCFunction)VectorArray, METH_VARARGS | METH_KEYWORDS /* unused */);
+#endif

// checked getitem
if (HasAttrDirect(pyclass, PyStrings::gLen)) {
--
2.43.0

1 change: 1 addition & 0 deletions bindings/pyroot/cppyy/sync-upstream
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ git apply patches/CPyCppyy-Always-convert-returned-std-string.patch
git apply patches/CPyCppyy-Disable-implicit-conversion-to-smart-ptr.patch
git apply patches/CPyCppyy-TString_converter.patch
git apply patches/CPyCppyy-Prevent-construction-of-agg-init-for-tuple.patch
git apply patches/CPyCppyy-Disable_std-vector-numpy-array-pythonization.patch
git apply patches/cppyy-No-CppyyLegacy-namespace.patch
git apply patches/cppyy-Remove-Windows-workaround.patch
git apply patches/cppyy-Don-t-enable-cling-autoloading.patch
2 changes: 1 addition & 1 deletion cmake/modules/SearchInstalledSoftware.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ endmacro()
# stop the configuration with a FATAL_ERROR in case of fail-on-missing=ON.
#----------------------------------------------------------------------------
macro(ROOT_CHECK_CONNECTION_AND_DISABLE_OPTION option_name)
ROOT_CHECK_CONNECTION("$(option_name)=OFF")
ROOT_CHECK_CONNECTION("${option_name}=OFF")
if(NO_CONNECTION)
message(STATUS "No internet connection, disabling '${option_name}' option")
set(${option_name} OFF CACHE BOOL "Disabled because there is no internet connection" FORCE)
Expand Down
3 changes: 2 additions & 1 deletion roofit/hs3/src/RooJSONFactoryWSTool.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,9 @@ std::string generate(const RooFit::JSONIO::ImportExpression &ex, const JSONNode
RooJSONFactoryWSTool::error(errMsg.str());
} else if (p[k].is_seq()) {
bool firstInner = true;
expression << "{";
for (RooAbsArg *arg : tool->requestArgList<RooAbsReal>(p, k)) {
expression << (firstInner ? "{" : ",") << arg->GetName();
expression << (firstInner ? "" : ",") << arg->GetName();
firstInner = false;
}
expression << "}";
Expand Down
7 changes: 4 additions & 3 deletions roofit/roofitcore/src/RooAbsPdf.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ double RooAbsPdf::extendedTerm(RooAbsData const& data, bool weightSquared, bool
* \f]
* <tr><td> `Range(double lo, double hi)` <td> Fit only data inside given range. A range named "fit" is created on the fly on all observables.
* <tr><td> `SumCoefRange(const char* name)` <td> Set the range in which to interpret the coefficients of RooAddPdf components
* <tr><td> `NumCPU(int num, int istrat)` <td> Parallelize NLL calculation on num CPUs
* <tr><td> `NumCPU(int num, int istrat)` <td> Parallelize NLL calculation on num CPUs. (Currently, this setting is ignored with the **cpu** Backend.)
* <table>
* <tr><th> Strategy <th> Effect
* <tr><td> 0 = RooFit::BulkPartition - *default* <td> Divide events in N equal chunks
Expand All @@ -853,10 +853,10 @@ double RooAbsPdf::extendedTerm(RooAbsData const& data, bool weightSquared, bool
* <tr><td> `EvalBackend(std::string const&)` <td> Choose a likelihood evaluation backend:
* <table>
* <tr><th> Backend <th> Description
* <tr><td> **cpu** - *default* <td> New vectorized evaluation mode, using faster math functions and auto-vectorisation.
* <tr><td> **cpu** - *default* <td> New vectorized evaluation mode, using faster math functions and auto-vectorisation (currently on a single thread).
* Since ROOT 6.23, this is the default if `EvalBackend()` is not passed, succeeding the **legacy** backend.
* If all RooAbsArg objects in the model support vectorized evaluation,
* likelihood computations are 2 to 10 times faster than with the **legacy** backend
* likelihood computations are 2 to 10 times faster than with the **legacy** backend (each on a single thread).
* - unless your dataset is so small that the vectorization is not worth it.
* The relative difference of the single log-likelihoods with respect to the legacy mode is usually better than \f$10^{-12}\f$,
* and for fit parameters it's usually better than \f$10^{-6}\f$. In past ROOT releases, this backend could be activated with the now deprecated `BatchMode()` option.
Expand All @@ -866,6 +866,7 @@ double RooAbsPdf::extendedTerm(RooAbsData const& data, bool weightSquared, bool
* This backend can drastically speed up the fit if all RooAbsArg object in the model support it.
* <tr><td> **legacy** <td> The original likelihood evaluation method.
* Evaluates the PDF for each single data entry at a time before summing the negative log probabilities.
* It supports multi-threading, but you might need more than 20 threads to maybe see about 10% performance gain over the default cpu-backend (that runs currently only on a single thread).
* <tr><td> **codegen** <td> **Experimental** - Generates and compiles minimal C++ code for the NLL on-the-fly and wraps it in the returned RooAbsReal.
* Also generates and compiles the code for the gradient using Automatic Differentiation (AD) with [Clad](https://github.com/vgvassilev/clad).
* This analytic gradient is passed to the minimizer, which can result in significant speedups for many-parameter fits,
Expand Down