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] Plots of nested RooSimultaneous ignore ProjWData #12652

Closed
1 task done
elusian opened this issue Apr 12, 2023 · 3 comments · Fixed by #12668
Closed
1 task done

[RF] Plots of nested RooSimultaneous ignore ProjWData #12652

elusian opened this issue Apr 12, 2023 · 3 comments · Fixed by #12668

Comments

@elusian
Copy link
Contributor

elusian commented Apr 12, 2023

  • Checked for duplicates

Describe the bug

Nested RooSimultaneous are flattened into a single RooSimultaneous with a RooSuperCategory as index.
When plotting a RooSimultaneous, if the index category is derived its servers are searched inside the ProjWData set, and if not found the plot becomes a slice in the categories.
However, the only server of a RooSuperCategory is the internal RooMultiCategory, so in case of a nested RooSimultaneous the plot is always a slice.
This is basically #12020, except during plots and not generation. The solution is probably the same.

Expected behavior

Categories are summed over and not sliced

To Reproduce

import ROOT

x = ROOT.RooRealVar('x', '', 0, 1)

c1 = ROOT.RooCategory('c1', '', {'c11': 0, 'c12': 1})
c2 = ROOT.RooCategory('c2', '', {'c21': 0})

u = ROOT.RooUniform('u', '', x)
g = ROOT.RooGaussian('g', '', x, ROOT.RooFit.RooConst(0.5), ROOT.RooFit.RooConst(0.1))
s1 = ROOT.RooSimultaneous('s1', '', {'c11': u, 'c12': g}, c1)
s2 = ROOT.RooSimultaneous('s2', '', {'c21': s1}, c2)

proto = ROOT.RooDataSet('proto', '', {c1, c2})
c1.setIndex(0)
for i in range(50):
    proto.add({c1, c2})
c1.setIndex(1)
for i in range(50):
    proto.add({c1, c2})
    
frame = x.frame()
s2.plotOn(frame, ProjWData = ({c1, c2}, proto)) # only gauss
c1.setIndex(0)
s2.plotOn(frame, ProjWData = ({c1, c2}, proto)) # only uniform
frame.Draw()

Setup

ROOT master from LCG dev3

Additional context

@elusian elusian added the bug label Apr 12, 2023
guitargeek added a commit to guitargeek/root that referenced this issue Apr 17, 2023
Just as with the generation of datasets from a RooSimultaneous, the
right way to iterate over the index category components is not to call
`servers()`, but to use `RooSimultaneous::flattenedCatList()`.

Closes root-project#12652.
@guitargeek
Copy link
Contributor

Thanks! Yes, the solution was exactly the same, and thanks to your reproducer I also found some other problem with nested RooSimultaneous pdfs that I fixed in the linked PR 👍

guitargeek added a commit that referenced this issue Apr 17, 2023
Just as with the generation of datasets from a RooSimultaneous, the
right way to iterate over the index category components is not to call
`servers()`, but to use `RooSimultaneous::flattenedCatList()`.

Closes #12652.
@guitargeek guitargeek added this to Issues in Fixed in 6.30/00 via automation Apr 17, 2023
@guitargeek
Copy link
Contributor

I forgot to mention, in your reproducer code there is also a small problem. You are only specifying how c1 and c2 are projected, not that they are projected. For this, you also need to add the Project keyword arg:

frame = x.frame()
s2.plotOn(frame, Project={c1, c2}, ProjWData = ({c1, c2}, proto), LineColor="r") # only gauss
c1.setIndex(0)
s2.plotOn(frame, Project={c1, c2}, ProjWData = ({c1, c2}, proto)) # only uniform
frame.Draw()

@elusian
Copy link
Contributor Author

elusian commented Apr 17, 2023

Thanks you for all the fixes!

You are of course correct about the reproducer bug. I am used to plotting the dataset too, since that sets the projection variables and categories for the frame. The example was too minimal in this case.

guitargeek added a commit to guitargeek/root that referenced this issue Apr 19, 2023
Just as with the generation of datasets from a RooSimultaneous, the
right way to iterate over the index category components is not to call
`servers()`, but to use `RooSimultaneous::flattenedCatList()`.

Closes root-project#12652.
guitargeek added a commit to guitargeek/root that referenced this issue Apr 21, 2023
Just as with the generation of datasets from a RooSimultaneous, the
right way to iterate over the index category components is not to call
`servers()`, but to use `RooSimultaneous::flattenedCatList()`.

Closes root-project#12652.
guitargeek added a commit that referenced this issue Apr 21, 2023
Just as with the generation of datasets from a RooSimultaneous, the
right way to iterate over the index category components is not to call
`servers()`, but to use `RooSimultaneous::flattenedCatList()`.

Closes #12652.
enirolf pushed a commit to enirolf/root that referenced this issue May 26, 2023
Just as with the generation of datasets from a RooSimultaneous, the
right way to iterate over the index category components is not to call
`servers()`, but to use `RooSimultaneous::flattenedCatList()`.

Closes root-project#12652.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants