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] Writing copy of RooWorkspace messes up proxies #12329

Closed
guitargeek opened this issue Feb 15, 2023 · 0 comments · Fixed by #12640
Closed

[RF] Writing copy of RooWorkspace messes up proxies #12329

guitargeek opened this issue Feb 15, 2023 · 0 comments · Fixed by #12640

Comments

@guitargeek
Copy link
Contributor

Take this workspace here:
workspace.root.txt

Now run this reproducer code:

void repro() {

   {
      std::unique_ptr<TFile> f{TFile::Open("workspace.root")};
      RooWorkspace* combined = f->Get<RooWorkspace>("combined"); // load the original
      cout << combined->function("gaus_x1y1_OSeMmuP_fakeOverall")->getProxy(0) << std::endl; // this is non-zero
      combined->writeToFile("another.root"); // supposed to create a nice copy
   }

   std::unique_ptr<TFile> f{TFile::Open("another.root")};
   RooWorkspace* w = f->Get<RooWorkspace>("combined"); // load the copy
   cout << w->function("gaus_x1y1_OSeMmuP_fakeOverall")->getProxy(0) << std::endl; // this is zero ???
}

The output will be:

0x55925a4e95c0
0

That's not good. When copying a workspace, the proxies should not be set to zero!

Note that the .txt suffix has to be removed from the input file. It's just there to be able to upload the workspace to GitHub.

Thanks to @will-cern for reporting this.

@guitargeek guitargeek self-assigned this Feb 15, 2023
guitargeek added a commit to guitargeek/root that referenced this issue Apr 11, 2023
In `TRefArray::GetObjectUID(int &uid, TObjet *obj)`, there is a code
branch for the case of an empty TRefArray and an object with a UID
assigned. Is this case a new process ID corresponding to the object is
assigned to the `TRefArray`. However, the `uid` output parameter is not
correctly assigned in this code branch, which is fixed in this commit.

The consequence of this bug is that the, since the `uid` output
parameter is not correctly assigned, that the implementation of
`TRefArray::Add*()` does not correctly work then.

Closes root-project#12329.
guitargeek added a commit to guitargeek/root that referenced this issue Apr 11, 2023
In `TRefArray::GetObjectUID(int &uid, TObjet *obj)`, there is a code
branch for the case of an empty TRefArray and an object with a UID
assigned. Is this case a new process ID corresponding to the object is
assigned to the `TRefArray`. However, the `uid` output parameter is not
correctly assigned in this code branch, which is fixed in this commit.

The consequence of this bug is that the, since the `uid` output
parameter is not correctly assigned, that the implementation of
`TRefArray::Add*()` does not correctly work then.

Closes root-project#12329.
guitargeek added a commit to guitargeek/root that referenced this issue Apr 11, 2023
In `TRefArray::GetObjectUID(int &uid, TObjet *obj)`, there is a code
branch for the case of an empty TRefArray and an object with a UID
assigned. Is this case a new process ID corresponding to the object is
assigned to the `TRefArray`. However, the `uid` output parameter is not
correctly assigned in this code branch, which is fixed in this commit.

The consequence of this bug is that the, since the `uid` output
parameter is not correctly assigned, that the implementation of
`TRefArray::Add*()` does not correctly work then.

Closes root-project#12329.
@guitargeek guitargeek added this to Issues in Fixed in 6.30/00 via automation Apr 11, 2023
guitargeek added a commit that referenced this issue Apr 11, 2023
In `TRefArray::GetObjectUID(int &uid, TObjet *obj)`, there is a code
branch for the case of an empty TRefArray and an object with a UID
assigned. Is this case a new process ID corresponding to the object is
assigned to the `TRefArray`. However, the `uid` output parameter is not
correctly assigned in this code branch, which is fixed in this commit.

The consequence of this bug is that the, since the `uid` output
parameter is not correctly assigned, that the implementation of
`TRefArray::Add*()` does not correctly work then.

Closes #12329.
guitargeek added a commit to guitargeek/root that referenced this issue Apr 19, 2023
In `TRefArray::GetObjectUID(int &uid, TObjet *obj)`, there is a code
branch for the case of an empty TRefArray and an object with a UID
assigned. Is this case a new process ID corresponding to the object is
assigned to the `TRefArray`. However, the `uid` output parameter is not
correctly assigned in this code branch, which is fixed in this commit.

The consequence of this bug is that the, since the `uid` output
parameter is not correctly assigned, that the implementation of
`TRefArray::Add*()` does not correctly work then.

Closes root-project#12329.
guitargeek added a commit that referenced this issue Apr 21, 2023
In `TRefArray::GetObjectUID(int &uid, TObjet *obj)`, there is a code
branch for the case of an empty TRefArray and an object with a UID
assigned. Is this case a new process ID corresponding to the object is
assigned to the `TRefArray`. However, the `uid` output parameter is not
correctly assigned in this code branch, which is fixed in this commit.

The consequence of this bug is that the, since the `uid` output
parameter is not correctly assigned, that the implementation of
`TRefArray::Add*()` does not correctly work then.

Closes #12329.
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.

1 participant