Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RF] Improving the interface to the batch mode in RooAbsPdf::fitTo and RooAbsPdf::createNLL #9420

Merged
merged 3 commits into from
Dec 12, 2021

Conversation

guitargeek
Copy link
Contributor

This is a follow-up to #9004.

Further description can be found in the commit messages.

The code that was previously in RooAbsPdf.cxx got moved to a new file
BatchModeHelpers.cxx. Furthermore, code duplication in retrieving the
configuration parameters is removed.
This makes the old batch mode interface available again in
`RooAbsPdf::fitTo` and `RooAbsPdf::createNLL`. It is not for the user,
but the developer to easily make performance comparisons. One just has
to pass the command argument `RooFit::BatchMode("old")`.

The enum class `RooBatchCompute::BatchMode` was also moved to
`RooFit::BatchModeOption` because it was never used int the batch
computation library. The `Option` was added to the name to avoid a
naming collision with the `RooFit::BatchMode` command argument.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu2004/soversion.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on mac11/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on mac1015/python3.
Running on macitois21.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Before, and exact foating point match was required in the
LikelihoodSerialTest.BatchedUnbinnedGaussianND unit test. However, the
results of the new batch mode can be slightly different numerically, so
we introduce a relative tolerance of 1e-14 when comparing the NLL from
the new likelihood classes to the batch mode NLL.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

Nice improvements in the interface for using batch mode computation!

@guitargeek guitargeek merged commit 9801694 into root-project:master Dec 12, 2021
@guitargeek guitargeek deleted the new_batchmode_followup_1 branch December 12, 2021 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants