Skip to content

Added correction factor for RooMultiPdf#19408

Merged
guitargeek merged 5 commits intoroot-project:masterfrom
GalinBistrev2:fix-root-3
Jul 23, 2025
Merged

Added correction factor for RooMultiPdf#19408
guitargeek merged 5 commits intoroot-project:masterfrom
GalinBistrev2:fix-root-3

Conversation

@GalinBistrev2
Copy link
Contributor

@GalinBistrev2 GalinBistrev2 commented Jul 18, 2025

In this PR I implement the correction factor for RooMultiPdf.This is done via virtual function override in the RooMultiPdf header file on line 23:
inline double getCorrection() const override {
return cFactor * static_cast<RooAbsReal *>(corr.at(x))->getVal(); }
This overrides a newly added virtual function in RooAbsPdf.h on line 180 called getCorrection. Than the correction factor is added to the NLL via a piece of code in the file FitHelpers.cxx.A gtest was created in order to asses the performence of the implementation.The test yields a result:

Running main() from ./googletest/src/gtest_main.cc
[==========] Running 3 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 1 test from RooMultiPdf
[ RUN      ] RooMultiPdf.SelectsCorrectPdf
[       OK ] RooMultiPdf.SelectsCorrectPdf (20 ms)
[----------] 1 test from RooMultiPdf (20 ms total)

[----------] 2 tests from RooMultiPdfTest
[ RUN      ] RooMultiPdfTest.FitConvergesAndReturnsReasonableResult
[#1] INFO:Fitting -- RooAbsPdf::fitTo(roomultipdf) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 200.272 μs
[#1] INFO:Fitting -- using generic CPU library compiled with no vectorizations
MathFunc call used
[#1] INFO:Fitting -- Function JIT time: 104.604 ms
[#1] INFO:Fitting -- Gradient generation time: 35.2268 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 50.992 ms
[FitHelpers] Detected correction term from RooAbsPdf::getCorrection(). Adding penalty to NLL.
[#1] INFO:Fitting -- RooAddition::defaultErrorLevel(nll_roomultipdf_gaus1Data_corrected) NOTE: No RooNLLVar or RooChi2Var found. Assuming RooNLLVarNew + penalty. Using 0.5.
Minuit2Minimizer: Minimize with max-calls 2500 convergence for edm < 1 strategy 0
Info in <Minuit2>: MnSeedGenerator Computing seed using NumericalGradient calculator
Info in <Minuit2>: MnSeedGenerator Evaluated function and gradient in 740.241 μs
Info in <Minuit2>: MnSeedGenerator Initial state: FCN =       1459.366472 Edm =       1.737536846 NCalls =     21
Info in <Minuit2>: NegativeG2LineSearch Doing a NegativeG2LineSearch since one of the G2 component is negative
Info in <Minuit2>: NegativeG2LineSearch Done after 5.524 μs
Info in <Minuit2>: MnSeedGenerator Negative G2 found - new state: 
  Minimum value : 1459.366472
  Edm           : 1.737536846
  Internal parameters:	[      1.126362609                0     -1.370461484     -1.370461484     -0.927595308]	
  Internal gradient  :	[                0     -268.0140978                0                0     -233.4581784]	
  Internal covariance matrix:
[[              2              0              0              0              0]
 [              0  2.0000002e-05              0              0              0]
 [              0              0              2              0              0]
 [              0              0              0              2              0]
 [              0              0              0              0  0.00010116038]]]
Info in <Minuit2>: MnSeedGenerator Initial state  
  Minimum value : 1459.366472
  Edm           : 1.737536846
  Internal parameters:	[      1.126362609                0     -1.370461484     -1.370461484     -0.927595308]	
  Internal gradient  :	[                0     -268.0140978                0                0     -233.4581784]	
  Internal covariance matrix:
[[              2              0              0              0              0]
 [              0  2.0000002e-05              0              0              0]
 [              0              0              2              0              0]
 [              0              0              0              2              0]
 [              0              0              0              0  0.00010116038]]]
Info in <Minuit2>: VariableMetricBuilder Start iterating until Edm is < 0.001 with call limit = 2500
Info in <Minuit2>: VariableMetricBuilder    0 - FCN =       1459.366472 Edm =       1.737536846 NCalls =     21
Info in <Minuit2>: VariableMetricBuilder    1 - FCN =       1457.596466 Edm =    0.003837972663 NCalls =     32
Info in <Minuit2>: VariableMetricBuilder    2 - FCN =       1457.592092 Edm =   1.001709351e-07 NCalls =     44
Info in <Minuit2>: VariableMetricBuilder Stop iterating after 747.091 μs
Minuit2Minimizer : Valid minimum - status = 0
FVAL  = 1457.59209165523339
Edm   = 1.0017093508385116e-07
Nfcn  = 44
expo_1	  = -0.01	 +/-  0.0414417	(limited)
mean1	  = 0.0268158	 +/-  0.0314805	(limited)
poly_1	  = 0.02	 +/-  0.450504	(limited)
poly_2	  = 0.02	 +/-  0.450504	(limited)
sigma1	  = 1.03785	 +/-  0.0231624	(limited)
[#1] INFO:Fitting -- RooAbsPdf::fitTo(gaus1_over_gaus1_Int[x]) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 95.332 μs
[#1] INFO:Fitting -- Function JIT time: 3.96151 ms
[#1] INFO:Fitting -- Gradient generation time: 5.13721 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 5.09295 ms
Minuit2Minimizer: Minimize with max-calls 1000 convergence for edm < 1 strategy 0
Info in <Minuit2>: MnSeedGenerator Using analytical (external) gradient calculator but cannot compute G2 - use then numerical gradient for G2
Info in <Minuit2>: MnSeedGenerator Computing seed using NumericalGradient calculator
Info in <Minuit2>: MnSeedGenerator Evaluated function and gradient in 136.667 μs
Info in <Minuit2>: MnSeedGenerator Initial state: FCN =       1457.866472 Edm =        1.73753676 NCalls =      9
Info in <Minuit2>: MnSeedGenerator Initial state  
  Minimum value : 1457.866472
  Edm           : 1.73753676
  Internal parameters:	[                0     -0.927595308]	
  Internal gradient  :	[     -268.0140978     -233.4581708]	
  Internal covariance matrix:
[[  2.0000002e-05              0]
 [              0  0.00010116038]]]
Info in <Minuit2>: VariableMetricBuilder Start iterating until Edm is < 0.001 with call limit = 1000
Info in <Minuit2>: VariableMetricBuilder    0 - FCN =       1457.866472 Edm =        1.73753676 NCalls =      9
Info in <Minuit2>: VariableMetricBuilder    1 - FCN =       1456.096466 Edm =    0.003837291546 NCalls =     10
Info in <Minuit2>: VariableMetricBuilder    2 - FCN =       1456.092092 Edm =   9.758576703e-08 NCalls =     12
Info in <Minuit2>: VariableMetricBuilder Stop iterating after 160.42 μs
Minuit2Minimizer : Valid minimum - status = 0
FVAL  = 1456.09209165383572
Edm   = 9.75857670292811742e-08
Nfcn  = 12
mean1	  = 0.0268158	 +/-  0.0314805	(limited)
sigma1	  = 1.03785	 +/-  0.0231622	(limited)
[#1] INFO:Fitting -- RooAbsPdf::fitTo(roomultipdf) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 134.173 μs
MathFunc call used
[#1] INFO:Fitting -- Function JIT time: 4.59295 ms
[#1] INFO:Fitting -- Gradient generation time: 13.7029 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 10.9684 ms
[FitHelpers] Detected correction term from RooAbsPdf::getCorrection(). Adding penalty to NLL.
[#1] INFO:Fitting -- RooAbsPdf::fitTo(gaus1_over_gaus1_Int[x]) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 100.191 μs
[#1] INFO:Fitting -- Function JIT time: 4.06867 ms
[#1] INFO:Fitting -- Gradient generation time: 5.25647 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 5.27841 ms
PDF index 0: n_param = 3, direct+penalty = 1457.59, multipdf = 1457.59
[#1] INFO:Fitting -- RooAbsPdf::fitTo(roomultipdf) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 145.234 μs
MathFunc call used
[#1] INFO:Fitting -- Function JIT time: 5.25541 ms
[#1] INFO:Fitting -- Gradient generation time: 13.7903 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 10.9747 ms
[FitHelpers] Detected correction term from RooAbsPdf::getCorrection(). Adding penalty to NLL.
[#1] INFO:Fitting -- RooAbsPdf::fitTo(polynomial_over_polynomial_Int[x]) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 94.017 μs
[#1] INFO:Fitting -- Function JIT time: 3.8232 ms
[#1] INFO:Fitting -- Gradient generation time: 7.42265 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 5.61707 ms
PDF index 1: n_param = 3, direct+penalty = 2997.23, multipdf = 2997.23
[#1] INFO:Fitting -- RooAbsPdf::fitTo(roomultipdf) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 149.005 μs
MathFunc call used
[#1] INFO:Fitting -- Function JIT time: 4.88803 ms
[#1] INFO:Fitting -- Gradient generation time: 13.9396 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 10.8782 ms
[FitHelpers] Detected correction term from RooAbsPdf::getCorrection(). Adding penalty to NLL.
[#1] INFO:Fitting -- RooAbsPdf::fitTo(exponential_over_exponential_Int[x]) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 90.43 μs
[#1] INFO:Fitting -- Function JIT time: 3.70766 ms
[#1] INFO:Fitting -- Gradient generation time: 4.70716 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 5.35141 ms
PDF index 2: n_param = 2, direct+penalty = 2998.67, multipdf = 2998.67
[       OK ] RooMultiPdfTest.FitConvergesAndReturnsReasonableResult (522 ms)
[ RUN      ] RooMultiPdfTest.PenaltyTermIsAppliedCorrectly
[#1] INFO:Fitting -- RooAbsPdf::fitTo(gauss1_over_gauss1_Int[x]) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 50.72 μs
[#1] INFO:Fitting -- Function JIT time: 3.45282 ms
[#1] INFO:Fitting -- Gradient generation time: 5.07162 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 5.48548 ms
[#1] INFO:Fitting -- RooAbsPdf::fitTo(multiPdf) fixing normalization set for coefficient determination to observables in data
[#1] INFO:Fitting -- Creation of NLL object took 145.852 μs
Ternary expression call used 
[#1] INFO:Fitting -- Function JIT time: 3.88576 ms
[#1] INFO:Fitting -- Gradient generation time: 5.77148 ms
[#1] INFO:Fitting -- Gradient IR to machine code time: 6.6782 ms
[FitHelpers] Detected correction term from RooAbsPdf::getCorrection(). Adding penalty to NLL.
NLL(gauss1):     137.342
NLL(multiPdf):   138.842
Expected penalty: 1.5
Delta:           1.5
[       OK ] RooMultiPdfTest.PenaltyTermIsAppliedCorrectly (31 ms)
[----------] 2 tests from RooMultiPdfTest (553 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 2 test suites ran. (574 ms total)
[  PASSED  ] 3 tests.

Also the header and the source file for the RooMultiPdf are moved to roofitcore in order to support an upcoming feature from CMS combine freezeParameters().

Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a few preliminary comments.

@GalinBistrev2 GalinBistrev2 marked this pull request as ready for review July 20, 2025 12:35
@GalinBistrev2 GalinBistrev2 marked this pull request as draft July 20, 2025 12:37
@guitargeek guitargeek marked this pull request as ready for review July 21, 2025 14:53
guitargeek and others added 5 commits July 23, 2025 00:54
Also have a `dynamic_cast` check if the pdfClone inside createNLLImpl is
actually a RooAbsPdf to prevent similar problems in the future.
Upstream from CMS Combine functions relevant for parameter freezing when
using RooMultiPdfs.
@github-actions
Copy link

Test Results

    21 files      21 suites   3d 19h 10m 42s ⏱️
 3 217 tests  3 216 ✅ 0 💤 1 ❌
65 834 runs  65 833 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit db17aeb.

Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the PR and addressing the review comments! I have also added the commit with the upstreamed helper functions from combine, that can now be included because you have moved the RooMultiPdf to RooFit core.

@guitargeek guitargeek merged commit 515ce21 into root-project:master Jul 23, 2025
23 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants