[RF] Avoid using transient std::unique_ptr
#15733
Conversation
Test Results 13 files 13 suites 3d 5h 2m 16s ⏱️ Results for commit ed22e9b. ♻️ This comment has been updated with latest results. |
|
From a practical point of view for RooFit, the fact that the problem is not understood should not block this PR to enter the next patch release, unless the underlying problem is fixed in the meantime. I'll try to create an easy reproducer and open a GitHub issue! |
|
The original issue (which was "fixed" by also removing the transient unique_ptr because it was blocking the transition to C++17) is at #13361 I couldn't create a standalone reproducer back then, also we should probably reintroduce the transient unique_ptr in the All of this to say, I agree with you, it's outside of scope for this PR |
Can you remind me how to reproduce this (as you saw it, no need to reduce)? |
|
You need to compile this code with the 3rd commit in the history reverted and run the tests (just like the CI does): But you need a ROOT build without RooFit for that ( |
|
Thanks. I am able reproduce that error. I am investigating. |
|
@vepadulano, @pcanal, this PR has already missed the 6.32.02 patch release. Can we merge it so at least this fix is sure to be in the next patch release? The investigation of the problem is independent of this PR I think. |
|
This PR is a 'downgrade' is code simplicity that should not be necessary. However the underlying fix is not ready (and won't until August), so we could temporarily merge this if need be. |
|
Ok, thanks for the comment! Let's see when the next patch release will be scheduled |
Harmless change to circumvent errors like these when building RooFit standalone: ``` Error in <TProtoClass::FindDataMember>: data member with index 0 is not found in class tuple<TreeReadBuffer*,default_delete<TreeReadBuffer> > Error in <CreateRealData>: Cannot find data member # 0 of class tuple<TreeReadBuffer*,default_delete<TreeReadBuffer> > for parent RooAddPdf! Error in <TProtoClass::FindDataMember>: data member with index 1 is not found in class tuple<TreeReadBuffer*,default_delete<TreeReadBuffer> > Error in <CreateRealData>: Cannot find data member # 1 of class tuple<TreeReadBuffer*,default_delete<TreeReadBuffer> > for parent RooAddPdf! Error in <TProtoClass::FindDataMember>: data member with index 0 is not found in class tuple<TreeReadBuffer*,default_delete<TreeReadBuffer> > Error in <CreateRealData>: Cannot find data member # 0 of class tuple<TreeReadBuffer*,default_delete<TreeReadBuffer> > for parent RooPolynomial! ```
guitargeek
force-pushed
the
transient_std_unique_ptr
branch
from
763d706 to
ed22e9b
Compare
|
Checking ... |
|
@guitargeek Potential good news. I am currently unable to reproduce the problem with the master nor with the tip of v6-32-00-patches. Can you please verify? Thanks. |
|
@guitargeek Nevermind. I just realized that I can still see the problem when running some of the test :( |
Do not complain about missing information about data members that directly or indirectly within a transient members of the top level class in GetRealData. This solves the underlying problem from root-project#15733
Do not complain about missing information about data members that directly or indirectly within a transient members of the top level class in GetRealData. This solves the underlying problem from root-project#15733
|
#16202 should fix this problem. Can you please check? |
|
I see the CI in that PR is still red. Should I wait for the PR to be updated such that it's green before I test? Or does it make sense to test already now? Thanks a lot already for fixing it 👍 |
|
It makes sense to test it now to know at the very least if my independent tests are sufficient or not. |
|
Perfect! It works! |
Do not complain about missing information about data members that directly or indirectly within a transient members of the top level class in GetRealData. This solves the underlying problem from root-project#15733
|
Superseded by a PR that fixes the underlying problem: |
Do not complain about missing information about data members that directly or indirectly within a transient members of the top level class in GetRealData. This solves the underlying problem from root-project#15733
|
Re-opening, because this should be merged in case #16202 doesn't get ready for the 6.34 release. |
Harmless change to circumvent errors like these when building RooFit standalone: