Skip to content

[RF,geom,TMVA] avoid shadowing parent class members #23062

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

Draft
ferdymercury wants to merge 4 commits into
base: master
Choose a base branch
from
+67 −86
Draft

[RF,geom,TMVA] avoid shadowing parent class members #23062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
77bea78
[RF] avoid shadowing/duplicating class members
ferdymercury Aug 10, 2026
6a16792
[tmva] rm unused variable that shadows
ferdymercury Aug 10, 2026
45eb6a1
[geom] rename or remove to avoid shadowing
ferdymercury Aug 10, 2026
086b2b0
Update RooAbsPdf.h
ferdymercury Aug 10, 2026
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
3 changes: 1 addition & 2 deletions geom/geom/inc/TGeoOpticalSurface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class TGeoOpticalSurface : public TNamed {
};

private:
std::string fName = ""; // Surface name
ESurfaceType fType = kTdielectric_metal; // Surface type
ESurfaceModel fModel = kMglisur; // Surface model
ESurfaceFinish fFinish = kFpolished; // Surface finish
Expand Down Expand Up @@ -162,7 +161,7 @@ class TGeoOpticalSurface : public TNamed {
static ESurfaceFinish StringToFinish(const char *finish);
static const char *FinishToString(ESurfaceFinish finish);

ClassDefOverride(TGeoOpticalSurface, 2) // Class representing an optical surface
ClassDefOverride(TGeoOpticalSurface, 3) // Class representing an optical surface
};

////////////////////////////////////////////////////////////////////////////
Expand Down
6 changes: 3 additions & 3 deletions geom/geom/inc/TGeoPatternFinder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,8 @@ class TGeoPatternHoneycomb : public TGeoPatternFinder {
// data members
Int_t fNrows; // number of rows
Int_t fAxisOnRows; // axis along each row
Int_t *fNdivisions; // [fNrows] number of divisions for each row
Double_t *fStart; // [fNrows] starting points for each row
Int_t *fNdivs; // [fNrows] number of divisions for each row
Double_t *fRowStart; // [fNrows] starting points for each row

@pcanal pcanal Aug 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not appear to be backward compatible in term of reading old file. Are we missing I/O customization rules? Can we add tests?

@linev linev Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original issue #8189 describes the problem.
Once duplicated name for class member introduced in the class - we have no easy way to make I/O conversion rule for it.
To resolve issue we need first to implement I/O conversion rules for such cases and only then modify mentioned classes.

@ferdymercury ferdymercury Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: to prevent new occurrences in the future, should we turn on this warning? https://stackoverflow.com/a/59047305/7471760

I have no idea about I/O conversion rules so I'll leave this PR as draft in case someone wants to take over ;)


protected:
TGeoPatternHoneycomb(const TGeoPatternHoneycomb &);
Expand All @@ -477,7 +477,7 @@ class TGeoPatternHoneycomb : public TGeoPatternFinder {
TGeoNode *FindNode(Double_t *point, const Double_t *dir = nullptr) override;
void UpdateMatrix(Int_t idiv, TGeoHMatrix &matrix) const override;

ClassDefOverride(TGeoPatternHoneycomb, 1) // pattern for honeycomb divisions
ClassDefOverride(TGeoPatternHoneycomb, 2) // pattern for honeycomb divisions
};

#endif
16 changes: 8 additions & 8 deletions geom/geom/src/TGeoPatternFinder.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2302,8 +2302,8 @@ TGeoPatternHoneycomb::TGeoPatternHoneycomb()
{
fNrows = 0;
fAxisOnRows = 0;
fNdivisions = nullptr;
fStart = nullptr;
fNdivs = nullptr;
fRowStart = nullptr;
CreateThreadData(1);
}
////////////////////////////////////////////////////////////////////////////////
Expand All @@ -2313,8 +2313,8 @@ TGeoPatternHoneycomb::TGeoPatternHoneycomb(TGeoVolume *vol, Int_t nrows) : TGeoP
{
fNrows = nrows;
fAxisOnRows = 0;
fNdivisions = nullptr;
fStart = nullptr;
fNdivs = nullptr;
fRowStart = nullptr;
CreateThreadData(1);
// compute everything else
}
Expand All @@ -2325,8 +2325,8 @@ TGeoPatternHoneycomb::TGeoPatternHoneycomb(const TGeoPatternHoneycomb &pfh)
: TGeoPatternFinder(pfh),
fNrows(pfh.fNrows),
fAxisOnRows(pfh.fAxisOnRows),
fNdivisions(pfh.fNdivisions),
fStart(pfh.fStart)
fNdivs(pfh.fNdivs),
fRowStart(pfh.fRowStart)
{
CreateThreadData(1);
}
Expand All @@ -2340,8 +2340,8 @@ TGeoPatternHoneycomb &TGeoPatternHoneycomb::operator=(const TGeoPatternHoneycomb
TGeoPatternFinder::operator=(pfh);
fNrows = pfh.fNrows;
fAxisOnRows = pfh.fAxisOnRows;
fNdivisions = pfh.fNdivisions;
fStart = pfh.fStart;
fNdivs = pfh.fNdivs;
fRowStart = pfh.fRowStart;
CreateThreadData(1);
}
return *this;
Expand Down
5 changes: 2 additions & 3 deletions roofit/roofit/inc/RooIntegralMorph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class RooIntegralMorph : public RooAbsCachedPdf {
void interpolateGap(Int_t ixlo, Int_t ixhi) ;

RooIntegralMorph* _self ; //
std::unique_ptr<RooArgSet> _nset ;
std::unique_ptr<RooArgSet> _nargset;
RooAbsPdf* _pdf1 ; // PDF1
RooAbsPdf* _pdf2 ; // PDF2
RooRealVar* _x ; // X
Expand Down Expand Up @@ -106,8 +106,7 @@ class RooIntegralMorph : public RooAbsCachedPdf {
double evaluate() const override ;

private:

ClassDefOverride(RooIntegralMorph,1) // Linear shape interpolation operator p.d.f
ClassDefOverride(RooIntegralMorph, 2) // Linear shape interpolation operator p.d.f
};

#endif
4 changes: 2 additions & 2 deletions roofit/roofit/inc/RooUnblindOffset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ class RooUnblindOffset : public RooAbsHiddenReal {
// Function evaluation
double evaluate() const override ;

RooRealProxy _value ;
RooRealProxy _proxyValue;
RooBlindTools _blindEngine ;

ClassDefOverride(RooUnblindOffset,1) // Offset unblinding transformation
ClassDefOverride(RooUnblindOffset, 2) // Offset unblinding transformation
};

#endif
4 changes: 2 additions & 2 deletions roofit/roofit/inc/RooUnblindPrecision.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ class RooUnblindPrecision : public RooAbsHiddenReal {
// Function evaluation
double evaluate() const override ;

RooRealProxy _value ; // Holder of the blind value
RooRealProxy _proxyValue; // Holder of the blind value
RooBlindTools _blindEngine ; // Blinding engine

ClassDefOverride(RooUnblindPrecision,1) // Precision unblinding transformation
ClassDefOverride(RooUnblindPrecision, 2) // Precision unblinding transformation
};

#endif
10 changes: 5 additions & 5 deletions roofit/roofit/src/RooIntegralMorph.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,12 +248,12 @@ RooIntegralMorph::MorphCacheElem::MorphCacheElem(RooIntegralMorph &self, const R
{
// Mark in base class that normalization of cached pdf is invariant under pdf parameters

_nset = std::make_unique<RooArgSet>(*_x);
_nargset = std::make_unique<RooArgSet>(*_x);

_c1 = std::unique_ptr<RooAbsReal>{_pdf1->createCdf(*_x)};
_c2 = std::unique_ptr<RooAbsReal>{_pdf2->createCdf(*_x)};
_cb1 = std::unique_ptr<RooAbsFunc>{_c1->bindVars(*_x,_nset.get())};
_cb2 = std::unique_ptr<RooAbsFunc>{_c2->bindVars(*_x,_nset.get())};
_cb1 = std::unique_ptr<RooAbsFunc>{_c1->bindVars(*_x,_nargset.get())};
_cb2 = std::unique_ptr<RooAbsFunc>{_c2->bindVars(*_x,_nargset.get())};

_rf1 = std::make_unique<RooBrentRootFinder>(*_cb1);
_rf2 = std::make_unique<RooBrentRootFinder>(*_cb2);
Expand Down Expand Up @@ -414,9 +414,9 @@ void RooIntegralMorph::MorphCacheElem::calculate(TIterator* dIter)
_rf2->findRoot(x2,x2,xMax,y) ;

_x->setVal(x1);
double f1x1 = _pdf1->getVal(_nset.get());
double f1x1 = _pdf1->getVal(_nargset.get());
_x->setVal(x2);
double f2x2 = _pdf2->getVal(_nset.get());
double f2x2 = _pdf2->getVal(_nargset.get());
double fbarX = f1x1*f2x2 / ( _alpha->getVal()*f2x2 + (1-_alpha->getVal())*f1x1 ) ;

dIter->Next() ;
Expand Down
10 changes: 5 additions & 5 deletions roofit/roofit/src/RooUnblindOffset.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ of the unblind parameter
RooUnblindOffset::RooUnblindOffset(const char *name, const char *title,
const char *blindString, double scale, RooAbsReal& cpasym)
: RooAbsHiddenReal(name,title),
_value("value","Offset blinded value",this,cpasym),
_proxyValue("value","Offset blinded value",this,cpasym),
_blindEngine(blindString,RooBlindTools::full,0.,scale)
{
}
Expand All @@ -51,7 +51,7 @@ RooUnblindOffset::RooUnblindOffset(const char *name, const char *title,
const char *blindString, double scale, RooAbsReal& cpasym,
RooAbsCategory& blindState)
: RooAbsHiddenReal(name,title,blindState),
_value("value","Offset blinded value",this,cpasym),
_proxyValue("value","Offset blinded value",this,cpasym),
_blindEngine(blindString,RooBlindTools::full,0.,scale)
{
}
Expand All @@ -61,7 +61,7 @@ RooUnblindOffset::RooUnblindOffset(const char *name, const char *title,

RooUnblindOffset::RooUnblindOffset(const RooUnblindOffset& other, const char* name) :
RooAbsHiddenReal(other, name),
_value("asym",this,other._value),
_proxyValue("asym",this,other._proxyValue),
_blindEngine(other._blindEngine)
{
}
Expand All @@ -73,9 +73,9 @@ double RooUnblindOffset::evaluate() const
{
if (isHidden()) {
// Blinding is active for this event
return _blindEngine.UnHideOffset(_value);
return _blindEngine.UnHideOffset(_proxyValue);
} else {
// Blinding is not active for this event
return _value ;
return _proxyValue ;
}
}
10 changes: 5 additions & 5 deletions roofit/roofit/src/RooUnblindPrecision.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ RooUnblindPrecision::RooUnblindPrecision(const char *name, const char *title,
double scale, RooAbsReal& value,
bool sin2betaMode)
: RooAbsHiddenReal(name,title),
_value("value","Precision blinded value",this,value),
_proxyValue("value","Precision blinded value",this,value),
_blindEngine(blindString,RooBlindTools::full,centralValue,scale,sin2betaMode)
{
}
Expand All @@ -54,7 +54,7 @@ RooUnblindPrecision::RooUnblindPrecision(const char *name, const char *title,
double scale, RooAbsReal& value, RooAbsCategory& blindState,
bool sin2betaMode)
: RooAbsHiddenReal(name,title,blindState),
_value("value","Precision blinded value",this,value),
_proxyValue("value","Precision blinded value",this,value),
_blindEngine(blindString,RooBlindTools::full,centralValue,scale,sin2betaMode)
{
}
Expand All @@ -64,7 +64,7 @@ RooUnblindPrecision::RooUnblindPrecision(const char *name, const char *title,

RooUnblindPrecision::RooUnblindPrecision(const RooUnblindPrecision& other, const char* name) :
RooAbsHiddenReal(other, name),
_value("asym",this,other._value),
_proxyValue("asym",this,other._proxyValue),
_blindEngine(other._blindEngine)
{
}
Expand All @@ -76,9 +76,9 @@ double RooUnblindPrecision::evaluate() const
{
if (isHidden()) {
// Blinding active for this event
return _blindEngine.UnHidePrecision(_value);
return _blindEngine.UnHidePrecision(_proxyValue);
} else {
// Blinding not active for this event
return _value ;
return _proxyValue ;
}
}
4 changes: 1 addition & 3 deletions roofit/roofitcore/inc/RooAbsPdf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,6 @@ class RooAbsPdf : public RooAbsReal {
mutable Int_t _traceCount = 0; ///< Number of traces remaining to print
mutable Int_t _negCount = 0; ///< Number of negative probabilities remaining to print

bool _selectComp = false; ///< Component selection flag for RooAbsPdf::plotCompOn

std::unique_ptr<RooNumGenConfig> _specGeneratorConfig ; ///<! MC generator configuration specific for this object

TString _normRange ; ///< Normalization range
Expand All @@ -342,7 +340,7 @@ class RooAbsPdf : public RooAbsReal {
friend class RooAbsReal;
friend class RooChi2Var;

ClassDefOverride(RooAbsPdf,5) // Abstract PDF with normalization support
ClassDefOverride(RooAbsPdf,6) // Abstract PDF with normalization support
};


Expand Down
29 changes: 12 additions & 17 deletions roofit/roofitcore/src/RooAbsOptTestStatistic.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,14 @@ parallelized calculation of test statistics.

#include "RooAbsOptTestStatistic.h"

#include "Riostream.h"
#include "TClass.h"
#include <cstring>

#include "RooAbsData.h"
#include "RooAbsDataStore.h"
#include "RooAbsPdf.h"
#include "RooAddPdf.h"
#include "RooArgSet.h"
#include "RooBinSamplingPdf.h"
#include "RooBinning.h"
#include "RooCategory.h"
#include "RooDataHist.h"
#include "RooDataSet.h"
#include "RooErrorHandler.h"
Expand All @@ -59,7 +55,6 @@ parallelized calculation of test statistics.
#include "RooMsgService.h"
#include "RooProdPdf.h"
#include "RooProduct.h"
#include "RooRealSumPdf.h"
#include "RooRealVar.h"
#include "RooVectorDataStore.h"

Expand Down Expand Up @@ -126,7 +121,7 @@ RooAbsOptTestStatistic::RooAbsOptTestStatistic(const RooAbsOptTestStatistic &oth
return;
}

initSlave(*other._funcClone, *other._dataClone, other._projDeps ? *other._projDeps : RooArgSet(),
initSlave(*other._funcClone, *other._dataClone, other._projDepsOpt ? *other._projDepsOpt : RooArgSet(),
other._rangeName.c_str(), other._addCoefRangeName.c_str());
}

Expand Down Expand Up @@ -297,16 +292,16 @@ void RooAbsOptTestStatistic::initSlave(RooAbsReal& real, RooAbsData& indata, con
// Remove projected dependents from normalization set
if (!projDeps.empty()) {

_projDeps = new RooArgSet;
projDeps.snapshot(*_projDeps, false) ;
_projDepsOpt = new RooArgSet;
projDeps.snapshot(*_projDepsOpt, false);

//RooArgSet* tobedel = (RooArgSet*) _normSet->selectCommon(*_projDeps) ;
_normSet->remove(*_projDeps,true,true) ;
// RooArgSet* tobedel = (RooArgSet*) _normSet->selectCommon(*_projDeps) ;
_normSet->remove(*_projDepsOpt, true, true);

// Mark all projected dependents as such
RooArgSet projDataDeps;
_funcObsSet->selectCommon(*_projDeps, projDataDeps);
projDataDeps.setAttribAll("projectedDependent") ;
// Mark all projected dependents as such
RooArgSet projDataDeps;
_funcObsSet->selectCommon(*_projDepsOpt, projDataDeps);
projDataDeps.setAttribAll("projectedDependent");
}


Expand Down Expand Up @@ -344,8 +339,8 @@ RooAbsOptTestStatistic::~RooAbsOptTestStatistic()
if (operMode()==Slave) {
delete _funcClone ;
delete _funcObsSet ;
if (_projDeps) {
delete _projDeps ;
if (_projDepsOpt) {
delete _projDepsOpt;
}
if (_ownData) {
delete _dataClone ;
Expand Down Expand Up @@ -769,7 +764,7 @@ const char* RooAbsOptTestStatistic::cacheUniqueSuffix() const {

void RooAbsOptTestStatistic::runRecalculateCache(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const
{
_dataClone->store()->recalculateCache(_projDeps, firstEvent, lastEvent, stepSize, _skipZeroWeights);
_dataClone->store()->recalculateCache(_projDepsOpt, firstEvent, lastEvent, stepSize, _skipZeroWeights);
}

/// \endcond
4 changes: 2 additions & 2 deletions roofit/roofitcore/src/RooAbsOptTestStatistic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class RooAbsOptTestStatistic : public RooAbsTestStatistic {
RooArgSet* _funcCloneSet = nullptr; ///< Set owning all components of internal clone of input function
RooAbsData* _dataClone = nullptr; ///< Pointer to internal clone if input data
RooAbsReal* _funcClone = nullptr; ///< Pointer to internal clone of input function
RooArgSet* _projDeps = nullptr; ///< Set of projected observable
bool _ownData = false; ///< Do we own the dataset
RooArgSet *_projDepsOpt = nullptr; ///< Set of projected observable (optimized)
bool _ownData = false; ///< Do we own the dataset
bool _sealed = false; ///< Is test statistic sealed -- i.e. no access to data
TString _sealNotice ; ///< User-defined notice shown when reading a sealed likelihood
RooArgSet* _funcObsSet = nullptr; ///< List of observables in the pdf expression
Expand Down
Loading
Loading