-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[RF] Don't double-scale pdfs plotted by RooSimultaneous::plotOn() #20618
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
base: master
Are you sure you want to change the base?
Conversation
After a8ef8b0, when plotting an extended pdf it automatically scales itself to the number of expected events. However, when plotting slices of a RooSimultaneous, the normalization is already precomputed in `RooSimultanous::plotOn()` and should not be overridden again. To prevent this, flag the calculated normatization as the final number of events, and not some relative factor, which it is by default. Closes root-project#20383.
It greatly helps debugging if the error message tells you which objects are actually existing.
In the documentation of the RooSimultaneous, it says about the `ProjWData()` argument: > For observables present in given dataset projection of PDF is achieved by constructing an average over all observable values in given set. And about `Slice()`, it says: > Override default projection behaviour by omitting the specified category observable from the projection, i.e., by not integrating over all states of this category. Starting from this explanation, it is highly unintuitive that one should do things like `simPdf.plotOn(Slice(sample, "state"), ProjWData(same))` when plotting the pdf of state `"state"` from a RooSimultaneous. I think that instead, we should promote easy patterns in the tutorials. That is, if you want to plot a slice pdf from a RooSimultaneous, you are retreiving it with `getPdf()` and then plot it. The result is the same.
Test Results 22 files 22 suites 4d 2h 17m 20s ⏱️ Results for commit 2517b0b. |
|
Dear @guitargeek And I like a lot significantly simplified "interface" without |
|
@guitargeek does this PR also clarify/close https://its.cern.ch/jira/browse/ROOT-7499 ? |
No. But I have created another PR for that: |
Just to be clear: the simplified interface is not new. Plotting the slices as I suggest now has always worked in RooFit, and I'm just changing the tutorials to use simpler patterns 🙂 Why the plot command in the tutorial was so convoluted is not clear to me. |
After a8ef8b0, when plotting an extended pdf it automatically scales
itself to the number of expected events.
However, when plotting slices of a RooSimultaneous, the normalization is
already precomputed in
RooSimultanous::plotOn()and should not beoverridden again. To prevent this, flag the calculated normatization as
the final number of events, and not some relative factor, which it is by
default.
Closes #20383.
The tutorials are also updated to use less confusing statements to plot slices of the RooSimultaneous.
@VanyaBelyaev, is that okay for you now?