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] RooDataSet columns added with addColumns method not copied correctly in reduce #8787

Closed
will-cern opened this issue Jul 31, 2021 · 2 comments · Fixed by #10504
Closed

Comments

@will-cern
Copy link
Contributor

Affects at least 6.22/08

Minimal reproducer:

{
    RooRealVar x("x","x",1);
    RooRealVar w("w","w",1);
    RooDataSet d("d","d",w,"w");

    d.addColumns(x);

    for(int i=0;i<10;i++) {
        x = i; d.add(x,0);
    }

    // d now has 10 entries with x values increasing
    for(int i=0;i<d.numEntries();i++) std::cout << d.get(i)->getRealValue("x") << ","; std::cout << std::endl;

    // now reduce the dataset and repeat:
    auto e = d.reduce("x>5");
    for(int i=0;i<e->numEntries();i++) std::cout << e->get(i)->getRealValue("x") << ","; std::cout << std::endl;
}

This outputs:

0,1,2,3,4,5,6,7,8,9,
9,9,9,9,

where it should really output:

0,1,2,3,4,5,6,7,8,9,
6,7,8,9,

If the "x" column was included when the dataset is constructed, the reduction works correctly.

@will-cern will-cern added the bug label Jul 31, 2021
@will-cern
Copy link
Contributor Author

incredibly, addColumn works ok though, so hopefully the problem is easy to track down by comparing those two methods.

I originally tried to workaround the problem by creating another dataset and merging it in. I discovered that suffers from another bug though that merged datasets lose their weightedness!

@guitargeek guitargeek changed the title RooDataSet columns added with addColumns method not copied correctly in reduce [RF] RooDataSet columns added with addColumns method not copied correctly in reduce Dec 3, 2021
guitargeek added a commit to guitargeek/root that referenced this issue May 2, 2022
Re-implementing the column adding functionality for multiple columns at
once is not worth it. Adding the columns one after the other has little
overhead, and the current implementation of
`RooVectorDataStore::addColumns` is buggy anyway (see root-project#8787).

Closes root-project#8787.
guitargeek added a commit to guitargeek/root that referenced this issue May 2, 2022
Re-implementing the column adding functionality for multiple columns at
once is not worth it. Adding the columns one after the other has little
overhead, and the current implementation of
`RooVectorDataStore::addColumns` is buggy anyway (see root-project#8787).

Closes root-project#8787.
guitargeek added a commit that referenced this issue May 3, 2022
Re-implementing the column adding functionality for multiple columns at
once is not worth it. Adding the columns one after the other has little
overhead, and the current implementation of
`RooVectorDataStore::addColumns` is buggy anyway (see #8787).

Closes #8787.
@github-actions
Copy link

github-actions bot commented May 4, 2022

Hi @guitargeek,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants