Skip to content

[RF] Add methods to create owning RooFit proxies via std::unique_ptr #12924

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.

Sign up for GitHub

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

Merged
guitargeek merged 1 commit into from
Aug 12, 2024
Merged

[RF] Add methods to create owning RooFit proxies via std::unique_ptr #12924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README/ReleaseNotes/v634/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ The following people have contributed to this new version:
## Deprecation and Removal

* The `RooAbsReal::plotSliceOn()` function that was deprecated since at least ROOT 6 was removed. Use `plotOn(frame,Slice(...))` instead.
* The `RooTemplateProxy` constructors that take a `proxyOwnsArg` parameter to manually pass ownership are deprecated and replaced by a new constructor that takes ownership via `std::unique_ptr<T>`. They will be removed in ROOT 6.36.

## Core Libraries

Expand Down
2 changes: 1 addition & 1 deletion roofit/roofit/src/RooNonCPEigenDecay.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ RooNonCPEigenDecay::RooNonCPEigenDecay( const char *name, const char *title,
_tag ( "tag", "CP state", this, tag ),
_rhoQ ( "rhoQ", "Charge of the rho", this, rhoQ ),
_correctQ ( "correctQ", "correction of rhoQ", this, correctQ ),
_wQ ( "wQ", "mischarge", this, *(new RooRealVar( "wQ", "wQ", 0 )), true, false, true ),
_wQ ( "wQ", "mischarge", this, std::make_unique<RooRealVar>( "wQ", "wQ", 0 ), true, false),
_genB0Frac ( 0 ),
_genRhoPlusFrac( 0 ),
_type ( type )
Expand Down
74 changes: 70 additions & 4 deletions roofit/roofitcore/inc/RooTemplateProxy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
#include "RooAbsRealLValue.h"
#include "RooAbsCategory.h"
#include "RooMsgService.h"

#include <ROOT/RConfig.hxx> // for the R__DEPRECATED macro

#include <string>

/**
Expand Down Expand Up @@ -159,11 +162,10 @@ class RooTemplateProxy : public RooArgProxy {
/// of client-server dependencies.
/// \param[in] valueServer Notify the owner if value changes.
/// \param[in] shapeServer Notify the owner if shape (e.g. binning) changes.
/// \param[in] proxyOwnsArg Proxy will delete the payload if owning.
template<typename Bool = bool, typename = std::enable_if_t<std::is_same<Bool,bool>::value>>
RooTemplateProxy(const char* theName, const char* desc, RooAbsArg* owner,
Bool valueServer=true, bool shapeServer=false, bool proxyOwnsArg=false)
: RooArgProxy(theName, desc, owner, valueServer, shapeServer, proxyOwnsArg) {
Bool valueServer=true, bool shapeServer=false)
: RooArgProxy(theName, desc, owner, valueServer, shapeServer, false) {
// Note for developers: the type of the first bool parameter is templated
// such that implicit conversion from int or pointers to bool is disabled.
// This is because there is another constructor with the signature
Expand All @@ -174,8 +176,53 @@ class RooTemplateProxy : public RooArgProxy {
// not happen.
}

#ifndef ROOFIT_MEMORY_SAFE_INTERFACES
/// Constructor with owner.
///
/// \deprecated Kept for backwards compatibility and will be removed in ROOT 6.36.
/// Either use RooTemplateProxy(const char*, const char*, RooAbsArg*, bool, bool),
/// and transfer the ownership with RooTemplateProxy::putOwnedArg(),
/// or use RooTemplateProxy(const char*, const char*, RooAbsArg*, std::unique_ptr<T>, bool, bool)
/// if you want the proxy to own the argument.
/// depending if you want to transfer ownership or not.
///
/// \param[in] theName Name of this proxy (for printing).
/// \param[in] desc Description what this proxy should act as.
/// \param[in] owner The object that owns the proxy. This is important for tracking
/// of client-server dependencies.
/// \param[in] valueServer Notify the owner if value changes.
/// \param[in] shapeServer Notify the owner if shape (e.g. binning) changes.
/// \param[in] proxyOwnsArg Proxy will delete the payload if owning.
template<typename Bool = bool, typename = std::enable_if_t<std::is_same<Bool,bool>::value>>
R__DEPRECATED(6,36, "Use RooTemplateProxy(const char*, const char*, RooAbsArg*, bool, bool) and transfer the ownership with RooTemplateProxy::putOwnedArg().")
RooTemplateProxy(const char* theName, const char* desc, RooAbsArg* owner,
Bool valueServer, bool shapeServer, bool proxyOwnsArg)
: RooArgProxy(theName, desc, owner, valueServer, shapeServer, proxyOwnsArg)
{}
#endif

////////////////////////////////////////////////////////////////////////////////
/// Constructor with owner and proxied object.
/// \param[in] theName Name of this proxy (for printing).
/// \param[in] desc Description what this proxy should act as.
/// \param[in] owner The object that owns the proxy. This is important for tracking
/// of client-server dependencies.
/// \param[in] ref Reference to the object that the proxy should hold.
/// \param[in] valueServer Notify the owner if value changes.
/// \param[in] shapeServer Notify the owner if shape (e.g. binning) changes.
RooTemplateProxy(const char* theName, const char* desc, RooAbsArg* owner, T& ref,
bool valueServer=true, bool shapeServer=false) :
RooArgProxy(theName, desc, owner, const_cast<typename std::remove_const<T>::type&>(ref), valueServer, shapeServer, false) { }

#ifndef ROOFIT_MEMORY_SAFE_INTERFACES
////////////////////////////////////////////////////////////////////////////////
/// Constructor with owner and proxied object.
///
/// \deprecated Kept for backwards compatibility and will be removed in ROOT 6.36.
/// Please use RooTemplateProxy(const char*, const char*, RooAbsArg*, std::unique_ptr<T>, bool, bool)
/// or RooTemplateProxy(const char*, const char*, RooAbsArg*, T&, bool, bool),
/// depending if you want to transfer ownership or not.
///
/// \param[in] theName Name of this proxy (for printing).
/// \param[in] desc Description what this proxy should act as.
/// \param[in] owner The object that owns the proxy. This is important for tracking
Expand All @@ -184,9 +231,28 @@ class RooTemplateProxy : public RooArgProxy {
/// \param[in] valueServer Notify the owner if value changes.
/// \param[in] shapeServer Notify the owner if shape (e.g. binning) changes.
/// \param[in] proxyOwnsArg Proxy will delete the payload if owning.
R__DEPRECATED(6,36, "Use constructors without proxyOwnsArg argument, taking the one that accepts a std::unique_ptr<T> if you want to pass ownership.")
RooTemplateProxy(const char* theName, const char* desc, RooAbsArg* owner, T& ref,
bool valueServer=true, bool shapeServer=false, bool proxyOwnsArg=false) :
bool valueServer, bool shapeServer, bool proxyOwnsArg) :
RooArgProxy(theName, desc, owner, const_cast<typename std::remove_const<T>::type&>(ref), valueServer, shapeServer, proxyOwnsArg) { }
#endif

////////////////////////////////////////////////////////////////////////////////
/// Constructor with owner and proxied object, taking ownership of the proxied object.
///
/// \param[in] theName Name of this proxy (for printing).
/// \param[in] desc Description what this proxy should act as.
/// \param[in] owner The object that owns the proxy. This is important for tracking
/// of client-server dependencies.
/// \param[in] ptr Owning smart pointer to the object that the proxy should hold. Ownership will be transferred to the proxy.
/// \param[in] valueServer Notify the owner if value changes.
/// \param[in] shapeServer Notify the owner if shape (e.g. binning) changes.
RooTemplateProxy(const char *theName, const char *desc, RooAbsArg *owner, std::unique_ptr<T> ptr,
bool valueServer = true, bool shapeServer = false)
: RooArgProxy(theName, desc, owner, *ptr, valueServer, shapeServer, true)
{
ptr.release();
}


////////////////////////////////////////////////////////////////////////////////
Expand Down
8 changes: 4 additions & 4 deletions roofit/roofitcore/src/RooNLLVarNew.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ constexpr const char *RooNLLVarNew::weightVarNameSumW2;
namespace {

// Use RooConstVar for dummies such that they don't get included in getParameters().
RooConstVar *dummyVar(const char *name)
std::unique_ptr<RooConstVar> dummyVar(const char *name)
{
return new RooConstVar(name, name, 1.0);
return std::make_unique<RooConstVar>(name, name, 1.0);
}

// Helper class to represent a template pdf based on the fit dataset.
Expand Down Expand Up @@ -128,8 +128,8 @@ RooNLLVarNew::RooNLLVarNew(const char *name, const char *title, RooAbsPdf &pdf,
bool isExtended, RooFit::OffsetMode offsetMode)
: RooAbsReal(name, title),
_pdf{"pdf", "pdf", this, pdf},
_weightVar{"weightVar", "weightVar", this, *dummyVar(weightVarName), true, false, true},
_weightSquaredVar{weightVarNameSumW2, weightVarNameSumW2, this, *dummyVar("weightSquardVar"), true, false, true},
_weightVar{"weightVar", "weightVar", this, dummyVar(weightVarName)},
_weightSquaredVar{weightVarNameSumW2, weightVarNameSumW2, this, dummyVar("weightSquardVar")},
_binnedL{pdf.getAttribute("BinnedLikelihoodActive")}
{
RooArgSet obs;
Expand Down
5 changes: 2 additions & 3 deletions roofit/roofitcore/src/RooNormalizedPdf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ class RooNormalizedPdf : public RooAbsPdf {
: _pdf("numerator", "numerator", this, pdf),
_normIntegral(
"denominator", "denominator", this,
*std::unique_ptr<RooAbsReal>{pdf.createIntegral(normSet, *pdf.getIntegratorConfig(), pdf.normRange())}
.release(),
true, false, true),
std::unique_ptr<RooAbsReal>{pdf.createIntegral(normSet, *pdf.getIntegratorConfig(), pdf.normRange())},
true, false),
_normSet{normSet}
{
auto name = std::string(pdf.GetName()) + "_over_" + _normIntegral->GetName();
Expand Down
6 changes: 2 additions & 4 deletions roofit/roofitcore/src/RooSuperCategory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ using std::endl, std::ostream;

ClassImp(RooSuperCategory);

RooSuperCategory::RooSuperCategory() : _multiCat("MultiCatProxy", "Stores a RooMultiCategory", this, true, true, true)
{
}
RooSuperCategory::RooSuperCategory() = default;

////////////////////////////////////////////////////////////////////////////////
/// Construct a super category from other categories.
Expand All @@ -57,7 +55,7 @@ RooSuperCategory::RooSuperCategory() : _multiCat("MultiCatProxy", "Stores a RooM
RooSuperCategory::RooSuperCategory(const char *name, const char *title, const RooArgSet& inputCategories) :
RooAbsCategoryLValue(name, title),
_multiCat("MultiCatProxy", "Stores a RooMultiCategory", this,
*new RooMultiCategory((std::string(name) + "_internalMultiCat").c_str(), title, inputCategories), true, true, true)
std::make_unique<RooMultiCategory>((std::string(name) + "_internalMultiCat").c_str(), title, inputCategories), true, true)
{
// Check category list
for (const auto arg : inputCategories) {
Expand Down