[v628][RF] Backports of RooFit PRs to v6-28-00-patches: Part 12
#12590
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t#11942) * [skip-ci][Windows] Add the prepareHistFactory.bat script * Delete an extra line
The `RooGrid` is a utility class for the `RooMCIntegrator`, which doesn't support IO itself. Therefore, it doesn't make sense to have a `ClassDef(1)` macro. It is only putting the unnecessary burden of keeping backwards compatibility on the developers. Therefore, this commit suggests to leave out the `ClassDef` macro out of `RooGrid`, and also remove the unnecessary base classes `TObject` and `RooPrintable`. There is only one printing function that makes sense anyway, which is kept without implementing the full `RooPrintable` interface.
The `RooAbsArg::redirectServers()` funciton has an optional flag to print an error if not all servers listed in the new server set were replaced. However, if one was already using this `mustReplaceAll` flag, it means that the error is intentional and should be considered fatal. An exception should be thrown with it.
guitargeek
requested review from
bellenot,
hageboeck and
lmoneta
as code owners
|
Starting build on |
6 tasks
The RooTruthModel can't be used in that test, because it will always return zero for negative observable values. The point of the tutorial is to demostrate that the fit can deal with negative amplitude values for negative observable values, so this feature of the RooTruthModel is not helpful here. It was only a coincidence that the tutorial still worked: there was a typo in the RooFormulaVars (extra whitespace), causing the RooTruthModel to just relay to the original formula instead of using the compiled code that returns zero for negative observable values.
The `RooTruthModel` when compiled for a basis registered only the `RooFormulaVar` for the basis as a value server, instead of replicating the server structore of the `RooFormulaVar` as done in the general `RooResolutionModel`. While this was the correct thing to do for the "generic basis" case where the RooFormulaVar was actually evaluated, it was wrong for the case where it is not evaluated (any other case). This commit changes the `RooTruthModel::changeBasis()` function such that in the non-generic basis case, it will take over the servers of the `RooFormulaVar`, just like in the implementation of the `RooResolutionModel` base class. This change is done to avoid unnecessary evaluation and copying in the RooFit GPU BatchMode. Also, a memory leak is fixed where the old basis was not deleted in case the resolution model owned the basis.
It is overly pedantic to ask from the user to pass the formula expression without whitespaces, therefore whitespaces are now removed from the input before matching the basis functions.
RooFit uses doubles everywhere else, so if importing a model from JSON should give the same biswise results as creating the model in the workspace factory language, `double` needs to be used in the JSON interface too.
It is better is the `STATIC_EXECUTE` macro doesn't take directly the lines of code to execute, because `clang-format` doesn't know what to do with it and formats the code wrongly.
Now, test many different pdfs in a new easily-extendible test suite, including the RooPolynomial for which support was added in the last commit.
This was extracted from the RooFitDriver to also be used elsewhere.
It would be nice if HistFactory propagated some additional information from the input objects to the workspace. For starters, the titles of observables could be taken from the axis of the input histograms, and channel and sample names from the titles of histograms. It would be good to think about how to best port over additional info (like histogram styling) once we have settled on a convention for that!
As a new RooFit source file, this file was supposed to be correctly formatted with the ROOT clang-format style. Since it was not correctly formatted and the next commit intends to change the file, it is first formatted in this commit.
The TList of filenames in the `HeatmapAnalyzer` is now correctly deleted using `std::unique_ptr`. Also, the following compiler warning is fixed: ``` roofit/multiprocess/src/HeatmapAnalyzer.cxx:52:22: warning: loop variable 'file' is always a copy because the range of type 'TList' does not return a reference [-Wrange-loop-analysis] ```
In the ROOT 6.26 development cycle, the RooProdPdf was partially rewritten in moden C++ with less manual memory allocation to improve performance (PR root-project#7907). In that PR, a unit test that verifies the RooProdPdf can correctly deal with factorizing PDFs was implemented. However, that test used an arbitrary PDF where the correct factorization was checked in a rather crude way: check by hashing the content of the RooProdPdf cache element for a given normalization set that said PR doesn't change any behavior (the reference hash was hardcoded in the unit test). This commit suggests a better alternative for the unit test, checking for a multidimensional product pdf of factorizing uniform pdfs that the pdf values for differenc normalization sets is as expected. This should cover the same functionality and is less fragile and implementation dependend than hashing the cache elements. This closes GitHub issue root-project#12430, as the rewritten test is not affected anymore by the problem reported in that issue.
guitargeek
force-pushed
the
v6-28-00-patches_roofit_backports_12
branch
from
6fa9c0b to
189aed0
Compare
|
Starting build on |
Contributor
Author
|
After merging this PR, the following commits in ROOT 41370dd378 [RF] Make it possible to use parameter titles in `RooAbsPdf::paramOn()`
912c32c5e2 [RF] Remove deprecated `Format(const char*, int)` command argument
3f925503b4 [RF] Fix memory leaks from `RooAbsL::getParameters()`
fb891723bc [RF][HS3] Avoid code duplication in `JSONFactories_RooFitCore`
9a605d7f35 [RF][HS3] Make `combined_data_name` optional
b87c368b6a [RF][HS3] Keep all info necessary to reconstruct simultaneous pdfs
f9348f857c [RF][HS3] Don't convert RooHistPdf first to TH1 when exporting
195d5b8111 [RF][HS3] Additions to JSONInterface to reduce code duplication
a75dec1868 [RF][HS3] Keep all information necessary to reconstruct combined datas
6b80645765 Revert "[RF] Make RooBatchCompute dependency public."
[RF] Move loop management for code generation into CodeSquashContext
[RF] Avoid need for buildLoopBegin() by recursive calls to translate()
[RF] Add 'translate' to RooNllVarNew.
[RF] Remove function declarations in RooStats LinkDef.h
[RF] Apply clang-format to ModelConfig.h and ModelConfig.cxx
[RF] Move `ModelConfig` from RooStats to RooFit
[RF] Don't add `weightVar` to observables in HistFactory
[RF] Minor improvements to RooFit evaluation code generation
[RF][NFC] Fix typo.
[RF] Disable RooFuncWrapper test if clad is off.
[RF] Remove wrong header declaration from roofit/roofit.
[RF] Fix visibility of the res/ directories.
[RF] Make RooBatchCompute dependency public.
[RF] Add initial interface and implementation for code-squashing.
[RF][HS3] Put exported `histosys` in the correct vector
[RF][HS3] Avoid creating temporary objects to import into workspace
[RF][HS3][HF] General cleanup of HS3 HistFactory implementation
[RF][HS3] Cover also `HistoSys` in HS3 HistFactory test
[RF] Enable passing of gradient function directly to RooMinimizer
[RF] Add support for differentiating Gaussian integrals using AD. This commits adds support for including analytical integrals into the mock code-squashing test by introducing a private header that stores the stateless implementation details.
[RF][HS3] Consistent range for nominal alpha params with HistFactory
[RF][HS3] Remember simultaneous channel names when writing JSON
[RF][HS3] Avoid turning RooConstVar into RooRealVar in JSON roundtrip
[RF][HS3] Use `RooConstVar` for sigma parameters in HF constraints
[RF][HS3] Don't mix up free functions and class impl in JSON tool
[RF][HS3] Consistently have implicit fallback for HistFactory variables
[RF][HS3] Don't import datasets that are parts of a combined dataset
[RF][HS3] Some code simplification in `RooJSONFactoryWSTool`
[RF][HS3] Generate input file for testHS3HistFactory on the fly
[RF][HS3] Improvements to the HS3 HistFactory implementation
[RF] Use `std::vector` diretly in RooVectorDataStore::RealFullVector
[RF][HS3] Some refactoring for less lines of code
[RF][HS3] Ordering fixes
[RF][HS3] Achieved closure for ATLAS ttW workspace
[RF][HS3] Sorting distributions
[RF][HS3] Bugfixes for histfactory workspaces
[RF][HS3] Small renamings
[RF][HS3] Improved attribute handling, caught some typecast-errors
[RF][HS3] Bugfix for FlexibleInterpVar
[RF] Avoid unnecessary warnings in `FlexibleInterpVar::setInterpCode`
[RF] Enable analytic integration of RooHistPdfs with RooLinearVars
[RF] Replace `RooAbsReal::_lastNSet` pointer with ID of last normSet
[RF] Remove `evaluateSpan()` from RooGenericPdf and RooFormulaVar
[RF][HS3] Re-retrieve element after exporting dependants
[RF][HS3] Don't write `histfactory_dist` axes redundantly
[RF] Exclude RooHistError from IO
[RF] Remove `add(row, weight, weightError)` from RooAbsData interface
[RF] Code-format `testRooDataHist.cxx`
[RF][HS3] Change analysis and likelihoods fields to match HS3 standard
[RF] Remove native buffers from `RooVectorDataStore::RealFullVector`
[RF] Modernize `RooVectorDataStore::RealFullVector` class
[RF][HS3] Move attributes and string attributes to `misc/ROOT_internal`
[RF][HS3] Support IO of models with RooConstVars
[RF][HS3] Don't do conversion to unbinned dataset manually in JSONIO
[RF] Enable AD code-gen test for RooFit.
[RF][HS3] Use arrays and not dicts to specify data axes
[RF][HS3] Remove exporter and importer function for RooProdPdf
[RF] Add function to clear node to RooFit JSON interface
[RF][HS3] Make "parameter_points" field conform with HS3 standard
[RF][HS3] Also migrate distributions and functions from dicts to arrays
[RF][HS3] Change some fields to use arrays and not dictionaries
[RF] Suggest alternative to RooDataSet c'tor that takes weight name
[RF] Add unit test for splitting RooDataSets with weight errors
[RF] Add weight errors and not weight squared when filling split data
[RF] Correctly propagate error storage in `RooDataSet::emptyClone()`
[RF][HS3] Don't assume that combined dataset name is always `"obsData"`
[RF][HS3] Use less `c_str()` conversions in RooFitHS3
[RF][HS3] New `wsEmplace()` method for creating objects in workspace
[RF][HS3] New `wsImport()` function to avoid repeating command args
[RF] Less manual memory management in RooAbsArg and RooProdGenContext
[RF] Code modernization of RooAbsReal
[RF][HS3] Renaming some distributions to conform with HS3 standard
[RF][HS3] Use HistFunc variables instead of underlying hist variables
[RF] Added protection against invalid variable names in createHistogram
[RF][HS3] Correct error messages when IO keys are missing
[RF][HS3] Code improvements in HS3 HistFactory
[RF][HS3] Small HS3 closure fixes
[RF][HS3] Import HistFactory constraints directly upon creation
[RF][HS3] Cleanup of generic functions to avoid using arguments
[RF][HS3] Don't import embedded data directly to RooWorkspace
[RF] Avoid false warnings in RooAbsReal::createHistogram()
[RF][HS3] Reduce verbosity of unit tests
[RF][HS3] Less usage of TString
[RF][HS3] Avoid code duplication when requesting RooArgLists and Sets
[RF][HS3] Remove unused functions from JSONFactories_HistFactory
[RF][HS3] Export `staterror` correctly for HistFactory
[RF][HS3] Major restructuring of HistFactory in HS3 - part 2
[RF][HS3] Major refactoring of `JSONFactories_HistFactory`
[RF][HS3] Correctly consider weight errors in `readBinnedData()`
[RF][HS3] Small code style improvement (renaming)
[RF][HS3] Make `testHS3HistFactory` less verbose
[RF][HS3] Changed some JSON keywords to comply with new HS3 standard
[RF][HS3] Moved `DependencyMissingError` to public to make it catchable
[RF][HS3] Support MultiVarGaussian and TF1Binding plus code cleanup
[RF][HS3] Added helper function for writing matrices in JSONInterface
[RF][HS3] JSON IO of positive definite flag in PiecewiseInterpolation
[RF] Added getters to TFnBinding, MultiVarGauss, PiecewiseInterpolation
[RF] Avoid invalid pointer access in `RooAbsPdf::printStream()`
[RF] Add intiial minimizer interface for RooFuncWrapper.
[RF] Improve code in `MinuitFcnGrad`
[RF] Code improvements in tests for new TestStatistics
[RF] Composition over inheritance in RooAbsMinimizerFcn implementations
[RF] No need for `RooAbsMinimizerFcn::fit()` method
[RF] Further refactor RooJSONFactoryWSTool
[RF][HS3] Avoid catching without re-throwing
[RF][HS3] Write out bin counts as `int` and other small improvements
[RF] Correctly support constants in RooFit JSON IO
[RF] Add test for a simultaneous model written and read from JSON
[RF] Skip the RooSimultaneous in JSON IO
[RF] Don't export `factory_tag` in JSON
[RF][HS3] Move the default variable values to the estimates section
[RF] RooFit JSON: no need to tag variables, just export them
[RF] Rename fields in JSON file to match new standard
[RF] Complete also the reading of likelihoods and analysis fields
[RF] Add likelihoods and analyses fields for HS3 v2
[RF] Decouple `RooDataHist` reading from rest of workspace in RooFitHS3
[RF] Some refactoring of RooFit HS3
[RF] New `domains` section in JSON to store variable ranges
Fix modules and modules.idx generation on Windows and disable a few more modules causing potential crashes (#12252)
[RF] Clearly mark `RooFit::CloneData()` as deprecated
[RF] Again code-format RooMinimizer
[RF] Avoid overhead of tracking evaluation error msgs when not needed
[RF] Implement safe way to set eval error logging in RooMinimizer
[RF] Use proper infinity checks in RooFit HS3
[RF] Avoid `sin` or `cos` of infinity in RooTruthModel integral code
[RF] RooTruthModel: local `xmin` and `xmax` to make code more readable
[RF] Define infinity as `std::numeric_limits<double>::infinity()`
[RF] Inline infinity constant and checks in `RooNumber`
[RF] Code-format RooNumber.h and RooNumber.cxx
[RF] Fill weight errors when importing RooDataHists to RooDataSet
[RF] Make it possible to directly import RooDataHist to RooDataSet
[RF] Reduce code duplication in RooDataSet constructor
[RF] Always create weight var in RooDataSet when importing weighted data
[RF] Implicitly create `WeightVar` for RooDataSet if it doesn't exist
[RF] Support also importing `RooDataHists` into RooDataSets
[RF] Avoid some code duplication in `RooDataSet` with importing slices
[RF] Split RooFuncWrapper into '.h' and '.cxx'.
[RF] Add observables as another parameter in RooFuncWrapper.
[RF] Test rough prototype of code generation in `testRooFuncWrapper`
Add AD based derivatives for RooFuncWrapper.
[cxxmodules] Enable a few modules for Windows. Now we can run hsimple.C.
[RF] Make it possible to switch to `ryml` backend after building ROOT
[RF] Add a C/C++ function wrapper class in roofit.
[RF] Fix implementation error from typo in `RooGenProdProj`
[RF] New mechanism to implicitly convert numbers to RooRealVar&
[RF] Remove RooFormula code for gcc <= 4.8
[RF] Remove `RooGenFunction` and `RooMultiGenFunction`
[RF] More use of `snapshot()` overload with output parameter
[RF] Bring back `RooStats::FeldmanCousins::SetData()`
[RF] Remove deprecated RooFit parts that are marked for removal in 6.30 |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backport of some RooFit PRs that were recently merged to master to v6-28-00-patches.
The final commit with the code modernization (others have already been backported in [v628][RF] Backports of RooFit PRs to
v6-28-00-patches: Part 1 #11960)RooGridclass from IO #11963RooSimultaneouswith proto data #12022Only the last one about throwing the exception in `RooAbsArg::redirectServers (the other commits have been backported already in [v628][RF] Backports of RooFit PRs to
v6-28-00-patches: Part 5 #12057 and [v628][RF] Backports of RooFit PRs tov6-28-00-patches: Part 7 #12092)RooTruthModeland related tutorials #12180doubleand notfloatall the way in RooFit JSON IO #12223Except for the first and last commit that relate to the RooFit AD work
auto const&゚Only the first two commits that were not already backported
RooProdPdf.TestGetPartIntListunit test #12447Related to #12319.