[6.38] [PyROOT] Fix a GIL deadlock in THx.Fit

bindings/distrdf/python/DistRDF/Backends/Dask/Backend.py

@@ -9,29 +9,29 @@
 # For the licensing terms see $ROOTSYS/LICENSE.                                #
 # For the list of contributors see $ROOTSYS/README/CREDITS.                    #
 ################################################################################
 from __future__ import annotations
 
 import os
 from typing import Any, Dict, List, Optional, Callable, TYPE_CHECKING, Union, Tuple
 import math
 import ROOT
 
 from DistRDF import DataFrame
 from DistRDF import HeadNode
 from DistRDF.Backends import Base
 from DistRDF.Backends import Utils
 
 try:
     import dask
     from dask.distributed import Client, get_worker, LocalCluster, progress, as_completed
 except ImportError:
     raise ImportError(("cannot import a Dask component. Refer to the Dask documentation "
                        "for installation instructions."))
 
 if TYPE_CHECKING:
     from dask_jobqueue import JobQueueCluster
     from DistRDF import Ranges
     from DistRDF._graph_cache import ExecutionIdentifier
 
 
 def get_total_cores_generic(client: Client) -> int:
@@ -259,7 +259,7 @@
         future_tasks = self.client.compute(mergeables_lists)
 
         # Save the current canvas
         backend_pad = ROOT.TVirtualPad.TContext()
 
         # Set up live visualization canvas
         c = self._setup_canvas(len(drawables_info_dict))
@@ -319,10 +319,11 @@
         cumulative_plots: Dict[int, Any] = {}
 
         # Collect all futures in batches that had arrived since the last iteration
-        for batch in as_completed(future_tasks, with_results=True).batches():
-            for future, result in batch:
-               merged_results = reducer(merged_results, result) if merged_results else result
-
+        for batch in as_completed(future_tasks, with_results=False).batches():
+            for future in batch:
+                result = future.result()
+                merged_results = reducer(merged_results, result) if merged_results else result
+
             mergeables = merged_results.mergeables
 
             for pad_num, (drawable_id, (callbacks_list, index, operation_name)) in enumerate(drawables_info_dict.items(), start=1):

bindings/pyroot/pythonizations/python/ROOT/_pythonization/_th1.py

@@ -265,3 +265,4 @@ def pythonize_th1(klass):
     _add_indexing_features(klass)
 
     inject_clone_releasing_ownership(klass)
+    klass.Fit.__release_gil__ = True

bindings/pyroot/pythonizations/test/CMakeLists.txt

@@ -59,7 +59,9 @@ ROOT_ADD_PYUNITTEST(pyroot_pyz_ttree_branch ttree_branch.py PYTHON_DEPS numpy)
 ROOT_ADD_PYUNITTEST(pyroot_pyz_tcolor tcolor.py)
 
 # TH1 and subclasses pythonizations
-ROOT_ADD_PYUNITTEST(pyroot_pyz_th1_operators th1_operators.py)
+ROOT_ADD_PYUNITTEST(pyroot_pyz_th1 th1.py)
+# The above tests a deadlock. It should complete in about 1s. If we don't reduce the timeout, we need to wait 1500 s.
+set_tests_properties(pyunittests-bindings-pyroot-pythonizations-pyroot-pyz-th1 PROPERTIES TIMEOUT 30)
 ROOT_ADD_PYUNITTEST(pyroot_pyz_th1_fillN th1_fillN.py PYTHON_DEPS numpy)
 ROOT_ADD_PYUNITTEST(pyroot_pyz_th2 th2.py)
 ROOT_ADD_PYUNITTEST(pyroot_pyz_th3 th3.py)

bindings/pyroot/pythonizations/test/th1.py

@@ -0,0 +1,70 @@
+import unittest
+
+import ROOT
+
+
+class TH1Operators(unittest.TestCase):
+    """
+    Test for the __imul__ operator of TH1 and subclasses, which
+    multiplies the histogram by a constant.
+    """
+
+    # Tests
+    def test_imul(self):
+        nbins = 64
+        h = ROOT.TH1F("testHist", "", nbins, -4, 4)
+        h.FillRandom("gaus")
+
+        initial_bins = [h.GetBinContent(i) for i in range(nbins)]
+        c = 2
+
+        # Multiply in place
+        h *= c
+
+        # Check new value of bins
+        for i in range(nbins):
+            self.assertEqual(h.GetBinContent(i), initial_bins[i] * c)
+
+
+class TH1IMT(unittest.TestCase):
+    """
+    Test a deadlock when IMT is used in conjunction with a fit function in Python.
+    Since TH1.Fit held the GIL, the fit function could never be evaluated
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        ROOT.ROOT.EnableImplicitMT(4)
+
+    @classmethod
+    def tearDownClass(cls):
+        ROOT.ROOT.DisableImplicitMT()
+
+    def test_fit_python_function(self):
+        xmin = 0
+        xmax = 1
+
+        h1 = ROOT.TH1F("h1", "", 20, xmin, xmax)
+        h1.FillRandom("gaus", 1000)
+
+        def func(x, pars):
+            return pars[0] + pars[1] * x[0]
+
+        my_func = ROOT.TF1("f1", func, xmin, xmax, npar=2, ndim=1)
+
+        my_func.SetParNames(
+            "A",
+            "B",
+        )
+        my_func.SetParameter(0, 1)
+        my_func.SetParameter(1, -1)
+
+        r = h1.Fit(my_func, "SE0Q", "", xmin, xmax)
+
+        self.assertFalse(r.IsEmpty())
+        self.assertTrue(r.IsValid())
+        self.assertGreater(r.Parameter(0), 0)
+
+
+if __name__ == "__main__":
+    unittest.main()