[RF] Add tutorial for simulation based inference #16044

robsyr · 2024-07-17T13:12:09Z

created new tutorial for simulation based inference (SBI)

github-actions · 2024-07-18T11:33:50Z

Test Results

13 files 13 suites 3d 2h 50m 52s ⏱️
3 028 tests 3 026 ✅ 0 💤 2 ❌
33 838 runs 33 836 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit d9e398f.

♻️ This comment has been updated with latest results.

README/ReleaseNotes/v634/index.md

bindings/pyroot/pythonizations/python/ROOT/_pythonization/_roofit/_rooglobalfunc.py

roofit/roofit/inc/RooPyBindFunction.h

roofit/roofit/src/RooPyBindFunction.cxx

roofit/roofit/inc/RooPyBindFunction.h

tutorials/roofit/rf615_learned_likelihoods.py

roofit/roofit/src/RooPyBindFunction.cxx

tutorials/roofit/rf615_learned_likelihoods.py

guitargeek

Thank you very much for this contribution. I have made a few change requests, but overall this is going in a very good direction!

Thanks to @bellenot, scikit-learn is now also installed on Windows 64 bit, but on Windows 32 bit it's not supported. Could you please add the tutorial to the list of tutorials that are vetoed on Windows 32 bit?

https://github.com/root-project/root/blob/master/tutorials/CMakeLists.txt#L131

Thanks a lot!

guitargeek · 2024-08-15T14:36:52Z

tutorials/CMakeLists.txt

@@ -138,7 +138,7 @@ else()
    # The following tutorials are failing with this error:
    # IncrementalExecutor::executeFunction: symbol '__std_find_trivial_4@12' unresolved while linking [cling interface function]!
    # on Windows 32 bit and Visual Studio v17.8
-    list(APPEND roofit_veto roofit/rf509_wsinteractive.C roofit/rf614_binned_fit_problems.C)
+    list(APPEND roofit_veto roofit/rf509_wsinteractive.C roofit/rf614_binned_fit_problems.C roofit/rf615_simulation_based_inference.py)


Suggested change

list(APPEND roofit_veto roofit/rf509_wsinteractive.C roofit/rf614_binned_fit_problems.C roofit/rf615_simulation_based_inference.py)

list(APPEND roofit_veto roofit/rf509_wsinteractive.C roofit/rf614_binned_fit_problems.C)

# The next tutorial uses scikit-learn, which is not available on Windows 32 bit

list(APPEND roofit_veto roofit/rf509_wsinteractive.C roofit/rf615_simulation_based_inference.py)

Now it looks like the comment above with the The following tutorials are failing with this... is also referring to you new tutorial, which is not the case. I would organize this differently.

guitargeek · 2024-08-15T14:37:30Z

roofit/roofit/src/RooPyBindFunction.cxx

+   m_varlist("!varlist", this, right.m_varlist) {
+}
+
+RooPyBindFunction::~RooPyBindFunction() {}


Suggested change

RooPyBindFunction::~RooPyBindFunction() {}

You already deleted the destructor declaration, but you also need to remove the corresponding implementation.

guitargeek · 2024-08-15T14:38:15Z

roofit/roofit/src/RooPyBindFunction.cxx

+
+#include "RooPyBindFunction.h"
+
+// ClassImp(RooPyBindFunction);


Suggested change

// ClassImp(RooPyBindFunction);

We should not push commented-out code if there is not strong reason for it

guitargeek · 2024-08-15T14:38:54Z

tutorials/roofit/rf615_simulation_based_inference.py

+## \macro_code
+## \macro_output


Suggested change

## \macro_code

## \macro_output

## \macro_image

## \macro_code

## \macro_output

We also want to see the output plots

guitargeek · 2024-08-15T14:39:35Z

roofit/roofit/src/RooPyBindFunction.cxx

+This clsss provides the functionality to wrap arbitrary python functions in
+RooFit. 
+*/
+


Suggested change

namespace RooFit {

namespace Detail {

The implementation needs to be in the same namespace (also think about the closing brackets at the end of the file).

guitargeek · 2024-08-15T14:41:11Z

tutorials/roofit/rf615_simulation_based_inference.py

@@ -0,0 +1,231 @@
+## \file


Can you go over the tutorial one with the code formatter?

black --line-length=120 tutorials/roofit/rf615_simulation_based_inference.py

guitargeek · 2024-08-15T14:42:29Z

bindings/pyroot/pythonizations/python/ROOT/_pythonization/_roofit/__init__.py

@@ -46,6 +46,7 @@
    LineStyle,
    FillStyle,
    MarkerStyle,
+    bindFunction


Please process this file with the code formatter.

guitargeek · 2024-08-15T14:42:36Z

bindings/pyroot/pythonizations/python/ROOT/_pythonization/_roofit/_rooglobalfunc.py

@@ -319,3 +319,37 @@ def DataError(etype):
            raise exception

    return RooFit._DataError(etype)
+
+


Please process this file with the code formatter.

guitargeek · 2024-08-15T14:43:58Z

tutorials/roofit/rf615_simulation_based_inference.py

+    # frame1= x_var.frame()
+    # c0= ROOT.TCanvas()
+    # frame1.Draw()
+    # input("")


Suggested change

# frame1= x_var.frame()

# c0= ROOT.TCanvas()

# frame1.Draw()

# input("")

# frame1 = x_var.frame()

# c0 = ROOT.TCanvas()

# frame1.Draw()

# input() # wait for user input to proceed

Also the commented-out code should be formatted :)

Oh, actually the part where you need to draw on the frame needs to be in the commented-out block too:

# Uncomment to see input plots # frame1 = x_var.frame() # for i in parampoints: # workspace[f"histpdf{i}"].plotOn(frame1) # c0 = ROOT.TCanvas() # frame1.Draw() # input() # wait for user input to proceed

guitargeek · 2024-08-15T14:44:43Z

tutorials/roofit/rf615_simulation_based_inference.py

+    workspace.Import(morph)
+
+
+# class used in this case to demonstate the use of SBI in Root


Suggested change

# class used in this case to demonstate the use of SBI in Root

# class used in this case to demonstrate the use of SBI in Root

guitargeek · 2024-08-15T14:45:08Z

tutorials/roofit/rf615_simulation_based_inference.py

+        self.classifier.fit(self.X_train, self.y_train)
+
+
+# Setting the training and toy data samples; the factor 5 to enable a fair comparisson to morphing


Suggested change

# Setting the training and toy data samples; the factor 5 to enable a fair comparisson to morphing

# Setting the training and toy data samples; the factor 5 to enable a fair comparison to morphing

guitargeek · 2024-08-15T14:46:19Z

tutorials/roofit/rf615_simulation_based_inference.py

+## \file
+## \ingroup tutorial_roofit
+## \notebook
+## Use Simulation Based Inference (SBI) in RooFit


Suggested change

## Use Simulation Based Inference (SBI) in RooFit

## Use Simulation Based Inference (SBI) in RooFit.

The short description is doxygen needs to end with a period

guitargeek · 2024-08-15T14:53:14Z

tutorials/roofit/rf615_simulation_based_inference.py

+    bin_mu_x = ROOT.RooBinning(4, 0., 4.)
+    grid = ROOT.RooMomentMorphFuncND.Grid2(bin_mu_x)


Suggested change

bin_mu_x = ROOT.RooBinning(4, 0., 4.)

grid = ROOT.RooMomentMorphFuncND.Grid2(bin_mu_x)

grid = ROOT.RooMomentMorphFuncND.Grid(ROOT.RooBinning(4, 0., 4.))

Can we do this in one line? And maybe use Grid instead of Grid2. I think Grid2 only exists for backwards compatibility, but it is identical.

guitargeek · 2024-08-15T14:54:33Z

tutorials/roofit/rf615_simulation_based_inference.py

+n_samples_train = n_samples * 5
+
+
+# define the "observed" data in a workspade


Suggested change

# define the "observed" data in a workspade

# define the "observed" data in a workspace

guitargeek · 2024-08-15T14:56:28Z

tutorials/roofit/rf615_simulation_based_inference.py

+# compute the "learned" negative log likelihood ratio
+nllr_learned = ROOT.RooFit.bindFunction("MyBinFunc", compute_log_likelihood_sum, mu_var)
+
+# compute the morphed nll


Suggested change

# compute the morphed nll

# Create the NLL based on the template morphing pdf

Sorry to be picky here, but to be consistent with the other RooFit tutorials, all the comments need to start with upper case. Also, we should try to be as precise as possible in the comments.

guitargeek · 2024-08-15T15:02:55Z

tutorials/roofit/rf615_simulation_based_inference.py

+    # number of 'sampled' gaussians, if you change it, adjust the binning properly
+    parampoints = np.arange(5)
+
+    for i in parampoints:


Suggested change

# number of 'sampled' gaussians, if you change it, adjust the binning properly

parampoints = np.arange(5)

for i in parampoints:

# number of 'sampled' gaussians, if you change it, adjust the binning properly

n_grid = 5

for i in range(n_grid):

No need to use numpy arrays here, especially not of type double

guitargeek · 2024-08-15T15:03:37Z

tutorials/roofit/rf615_simulation_based_inference.py

+        # pdf = workspace[f'histpdf{i}']
+
+        # add the pdf to the grid
+        grid.addPdf(workspace[f'histpdf{i}'], int(i))


Suggested change

grid.addPdf(workspace[f'histpdf{i}'], int(i))

grid.addPdf(workspace[f'histpdf{i}'], i)

If you use a simple integer index instead of the NumPy array element (which is a double), I think you also won't need the conversion.

guitargeek

Thanks for the updates, I think we're very close! Can you please also format the .h and .cxx files with the clang-format style of ROOT? Thanks!
https://github.com/root-project/root/blob/master/.clang-format

- added new tutorial for simulation based inference with roofit - added RooPyBindFunction to bind python function with arbitrary number of parameters - added Realease notes about previous changes

guitargeek

Thanks a lot for this new tutorial and following up on all the change requests.

Congrats to your first contribution!

robsyr requested a review from couet as a code owner July 17, 2024 13:12

dpiparo requested a review from guitargeek July 18, 2024 09:31

dpiparo assigned robsyr Jul 18, 2024

robsyr requested review from bellenot and lmoneta as code owners July 19, 2024 15:15

robsyr requested review from linev, dpiparo, jblomer and pcanal as code owners July 19, 2024 15:39

robsyr force-pushed the dev_llh_tut branch from d587134 to 3f105d0 Compare July 26, 2024 09:47

guitargeek changed the title ~~Add tutorial SBI~~ [RF] Add tutorial SBI Jul 29, 2024

guitargeek removed request for jblomer, couet, linev, pcanal, lmoneta, bellenot and dpiparo July 29, 2024 13:40

guitargeek added the in:RooFit/RooStats label Aug 9, 2024

robsyr force-pushed the dev_llh_tut branch from af02183 to f59fcf8 Compare August 12, 2024 09:25

robsyr requested a review from vepadulano as a code owner August 12, 2024 12:17