Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions README/ReleaseNotes/v642/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,21 @@ Users are strongly encouraged to switch to the vectorized CPU backend if they ar

If the vectorized backend does not work for a given use case, **please report it by opening an issue on the ROOT GitHub repository**.

### Default binning of RooFit variables changed to zero bins

A freshly-constructed `RooRealVar` (or `RooErrorVar`) no longer has a default binning of 100 bins.
Instead, `RooAbsRealLValue::getBins()` now returns `0` until a binning is explicitly set (e.g. via `RooRealVar::setBins()`).
This makes it possible to distinguish a variable whose binning was deliberately chosen from one that was left at the default, which avoids
writing redundant `nbins` fields when serializing workspaces to HS3 JSON.

For the cases that previously relied on the default of 100 bins, that value is now injected by the relevant routine:
unbinned-dataset plotting (`RooAbsRealLValue::frame()`), `RooAbsPdf::generateBinned()`, and `RooAbsRealLValue::createHistogram()`
all fall back to `RooAbsRealLValue::DefaultNBins` (100) when the variable has no binning set.
As a result, plotting, generating binned data and creating histograms from a default-constructed variable behave exactly as before.

Code that reads `getBins()`/`numBins()` of a bare variable and expected the value `100` should either set the binning explicitly,
or read the bin count from the relevant histogram or plot frame instead.

## Graphics and GUI

### Store canvas as HTML file
Expand Down
2 changes: 1 addition & 1 deletion roofit/hs3/src/RooJSONFactoryWSTool.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ void RooJSONFactoryWSTool::exportVariable(const RooAbsArg *v, JSONNode &node, bo
var["min"] << rrv->getMin();
var["max"] << rrv->getMax();
}
if (rrv->getBins() != 100 && storeBins) {
if (rrv->getBins() != 0 && storeBins) {
var["nbins"] << rrv->getBins();
}
_domains->readVariable(*rrv);
Expand Down
6 changes: 6 additions & 0 deletions roofit/roofitcore/inc/RooAbsRealLValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ class RooAbsRealLValue : public RooAbsReal, public RooAbsLValue {
/// Get number of bins of currently defined range.
/// \param name Optionally, request number of bins for range with given name.
virtual Int_t getBins(const char* name=nullptr) const { return getBinning(name).numBins(); }

/// Historical default number of bins, injected by routines that need a
/// concrete bin count when a variable has no binning explicitly set
/// (i.e. when getBins() returns 0). Used for unbinned-data plotting,
/// generateBinned() and createHistogram().
static constexpr int DefaultNBins = 100;
/// Get minimum of currently defined range.
/// \param name Optionally, request minimum of range with given name.
virtual double getMin(const char* name=nullptr) const { return getBinning(name).lowBound(); }
Expand Down
10 changes: 5 additions & 5 deletions roofit/roofitcore/inc/RooUniformBinning.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class RooUniformBinning : public RooAbsBinning {
public:

RooUniformBinning(const char* name=nullptr) : RooAbsBinning{name} {}
RooUniformBinning(double xlo, double xhi, Int_t nBins, const char* name=nullptr) ;
RooUniformBinning(double xlo, double xhi, Int_t nBins=0, const char* name=nullptr) ;
RooUniformBinning(const RooUniformBinning& other, const char* name=nullptr) ;
RooAbsBinning* clone(const char* name=nullptr) const override { return new RooUniformBinning(*this,name?name:GetName()) ; }

Expand All @@ -48,10 +48,10 @@ class RooUniformBinning : public RooAbsBinning {

protected:
mutable std::vector<double> _array; ///<! do not persist
double _xlo;
double _xhi;
Int_t _nbins;
double _binw;
double _xlo = 0.0;
double _xhi = 0.0;
Int_t _nbins = 0;
double _binw = 0.0;

ClassDefOverride(RooUniformBinning, 1) // Uniform binning specification
};
Expand Down
4 changes: 3 additions & 1 deletion roofit/roofitcore/src/RooAbsData.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -2543,7 +2543,9 @@ TH2F *RooAbsData::createHistogram(const RooAbsRealLValue &var1, const RooAbsReal
const char *name) const
{
checkInit();
return createHistogram(var1, var2, var1.getBins(), var2.getBins(), cuts, name);
const int nBins1 = var1.getBins()!=0 ? var1.getBins() : RooAbsRealLValue::DefaultNBins;
const int nBins2 = var2.getBins()!=0 ? var2.getBins() : RooAbsRealLValue::DefaultNBins;
return createHistogram(var1, var2, nBins1, nBins2, cuts, name);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
30 changes: 21 additions & 9 deletions roofit/roofitcore/src/RooAbsRealLValue.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ RooPlot* RooAbsRealLValue::frame(const RooLinkedList& cmdList) const
RooCmdConfig pc("RooAbsRealLValue::frame(" + std::string(GetName()) + ")");
pc.defineDouble("min","Range",0,getMin()) ;
pc.defineDouble("max","Range",1,getMax()) ;
pc.defineInt("nbins","Bins",0,getBins()) ;
pc.defineInt("nbins","Bins",0, getBins()!=0 ? getBins() : DefaultNBins) ;
pc.defineString("rangeName","RangeWithName",0,"") ;
pc.defineString("name","Name",0,"") ;
pc.defineString("title","Title",0,"") ;
Expand Down Expand Up @@ -316,7 +316,7 @@ RooPlot *RooAbsRealLValue::frame(double xlo, double xhi, Int_t nbins) const

RooPlot *RooAbsRealLValue::frame(double xlo, double xhi) const
{
return new RooPlot(*this,xlo,xhi,getBins());
return new RooPlot(*this,xlo,xhi, getBins()!=0 ? getBins() : DefaultNBins);
}


Expand Down Expand Up @@ -366,7 +366,7 @@ RooPlot *RooAbsRealLValue::frame() const
return nullptr ;
}

return new RooPlot(*this,getMin(),getMax(),getBins());
return new RooPlot(*this,getMin(),getMax(), getBins()!=0 ? getBins() : DefaultNBins);
}


Expand Down Expand Up @@ -601,6 +601,10 @@ TH1* RooAbsRealLValue::createHistogram(const char *name, const RooLinkedList& cm
ownBinning[0] = true ;
} else {
binning[0] = &getBinning() ;
if (binning[0]->numBins() == 0) {
binning[0] = new RooUniformBinning(getMin(), getMax(), DefaultNBins) ;
ownBinning[0] = true ;
}
}

if (pc.hasProcessed("YVar")) {
Expand All @@ -617,6 +621,10 @@ TH1* RooAbsRealLValue::createHistogram(const char *name, const RooLinkedList& cm
ownBinning[1] = true ;
} else {
binning[1] = &yvar.getBinning() ;
if (binning[1]->numBins() == 0) {
binning[1] = new RooUniformBinning(yvar.getMin(), yvar.getMax(), DefaultNBins) ;
ownBinning[1] = true ;
}
}
}

Expand All @@ -634,6 +642,10 @@ TH1* RooAbsRealLValue::createHistogram(const char *name, const RooLinkedList& cm
ownBinning[2] = true ;
} else {
binning[2] = &zvar.getBinning() ;
if (binning[2]->numBins() == 0) {
binning[2] = new RooUniformBinning(zvar.getMin(), zvar.getMax(), DefaultNBins) ;
ownBinning[2] = true ;
}
}
}

Expand Down Expand Up @@ -668,7 +680,7 @@ TH1F *RooAbsRealLValue::createHistogram(const char *name, const char *yAxisLabel
RooArgList list(*this) ;
double xlo = getMin() ;
double xhi = getMax() ;
Int_t nbins = getBins() ;
Int_t nbins = getBins()!=0 ? getBins() : DefaultNBins ;

// coverity[ARRAY_VS_SINGLETON]
return static_cast<TH1F*>(createHistogram(name, list, yAxisLabel, &xlo, &xhi, &nbins));
Expand Down Expand Up @@ -755,8 +767,8 @@ TH2F *RooAbsRealLValue::createHistogram(const char *name, const RooAbsRealLValue
}

if (!nBins2) {
nbins_fit[0] = getBins() ;
nbins_fit[1] = yvar.getBins() ;
nbins_fit[0] = getBins()!=0 ? getBins() : DefaultNBins ;
nbins_fit[1] = yvar.getBins()!=0 ? yvar.getBins() : DefaultNBins ;
nBins2 = nbins_fit ;
}

Expand Down Expand Up @@ -836,9 +848,9 @@ TH3F *RooAbsRealLValue::createHistogram(const char *name, const RooAbsRealLValue
}

if (!nBins2) {
nbins_fit[0] = getBins() ;
nbins_fit[1] = yvar.getBins() ;
nbins_fit[2] = zvar.getBins() ;
nbins_fit[0] = getBins()!=0 ? getBins() : DefaultNBins ;
nbins_fit[1] = yvar.getBins()!=0 ? yvar.getBins() : DefaultNBins ;
nbins_fit[2] = zvar.getBins()!=0 ? zvar.getBins() : DefaultNBins ;
nBins2 = nbins_fit ;
}

Expand Down
20 changes: 19 additions & 1 deletion roofit/roofitcore/src/RooDataHist.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,17 @@ void RooDataHist::initialize(const char* binningName, bool fillTree)
}
}

// If the variable has no binning explicitly set (the default for a
// freshly-constructed RooRealVar, which reports zero bins), materialize the
// historical default binning. _vars holds this dataset's own clones (see
// RooAbsData::initializeVars, which addClone's the input variables), so this
// does not affect the user's original variable.
if (RooRealVar* rrv = dynamic_cast<RooRealVar*>(_vars[i])) {
if (rrv->getBins() == 0) {
rrv->setBinning(RooUniformBinning(rrv->getMin(), rrv->getMax(), RooAbsRealLValue::DefaultNBins));
}
}

auto lvarg = dynamic_cast<RooAbsLValue*>(_vars[i]);
assert(lvarg);
_lvvars.push_back(lvarg);
Expand Down Expand Up @@ -1485,7 +1496,14 @@ double RooDataHist::weightInterpolated(const RooArgSet& bin, int intOrder, bool
double xval = realX.getVal() ;
double yval = realY.getVal() ;

RooAbsBinning const& binningY = realY.getBinning();
// Use the internal binning of the y variable, not the binning of the
// variable passed in `bin`. The latter may be a different object than the
// one owned by this RooDataHist (e.g. the histogram observable clone of a
// RooHistPdf), with an unrelated default binning. The bin indexing below
// relies on `_idxMult` and `centralIdx`, which are both expressed in terms
// of the internal binning, so the y binning must match it too. This mirrors
// what calcTreeIndex() and interpolateDim() do for the other dimensions.
RooAbsBinning const& binningY = static_cast<RooRealVar const&>(*_vars[varInfo.realVarIdx2]).getBinning();

int ybinC = binningY.binNumber(yval) ;
int ybinLo = ybinC-intOrder/2 - ((yval<binningY.binCenter(ybinC))?1:0) ;
Expand Down
2 changes: 1 addition & 1 deletion roofit/roofitcore/src/RooErrorVar.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ RooErrorVar::RooErrorVar(const char *name, const char *title, const RooRealVar&
RooAbsRealLValue(name,title),
_realVar("realVar","RooRealVar with error",this,(RooAbsReal&)input)
{
_binning = std::make_unique<RooUniformBinning>(-1,1,100) ;
_binning = std::make_unique<RooUniformBinning>(-1,1) ;
}


Expand Down
18 changes: 14 additions & 4 deletions roofit/roofitcore/src/RooHist.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ a RooPlot.
#include "RooHist.h"

#include "RooAbsRealLValue.h"
#include "RooUniformBinning.h"
#include "RooHistError.h"
#include "RooCurve.h"
#include "RooMsgService.h"
Expand Down Expand Up @@ -302,13 +303,22 @@ RooHist::RooHist(const RooAbsReal &f, RooAbsRealLValue &x, double xErrorFrac, do
RooProduct scaledFunc{"scaled_func", "scaled_func", {f, RooFit::RooConst(scaleFactor)}};
std::unique_ptr<RooAbsFunc> funcPtr{scaledFunc.bindVars(x, normVars, true)};

// calculate the points to add to our curve
int xbins = x.numBins();
// calculate the points to add to our curve. A variable with no binning
// explicitly set reports zero bins; in that case fall back to the historical
// default bin count for plotting, using a local binning so that the user's
// variable is not mutated.
const RooAbsBinning *xbinning = &x.getBinning();
std::unique_ptr<RooUniformBinning> defaultBinning;
if (x.numBins() == 0) {
defaultBinning = std::make_unique<RooUniformBinning>(x.getMin(), x.getMax(), RooAbsRealLValue::DefaultNBins);
xbinning = defaultBinning.get();
}
int xbins = xbinning->numBins();
RooArgSet nset;
if(normVars) nset.add(*normVars);
for(int i=0; i<xbins; ++i){
double xval = x.getBinning().binCenter(i);
double xwidth = x.getBinning().binWidth(i);
double xval = xbinning->binCenter(i);
double xwidth = xbinning->binWidth(i);
Axis_t xval_ax = xval;
double yval = (*funcPtr)(&xval);
double yerr = std::sqrt(yval);
Expand Down
10 changes: 5 additions & 5 deletions roofit/roofitcore/src/RooRealVar.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ RooRealVar::RooRealVar() : _error(0), _asymErrLo(0), _asymErrHi(0), _binning(n
RooRealVar::RooRealVar(const char *name, const char *title,
double value, const char *unit) :
RooAbsRealLValue(name, title, unit), _error(-1), _asymErrLo(1), _asymErrHi(-1),
_binning(new RooUniformBinning(-1,1,100))
_binning(new RooUniformBinning(-1,1))
{
_value = value ;
_fast = true ;
Expand All @@ -122,7 +122,7 @@ RooRealVar::RooRealVar(const char *name, const char *title,
double minValue, double maxValue,
const char *unit) :
RooAbsRealLValue(name, title, unit), _error(-1), _asymErrLo(1), _asymErrHi(-1),
_binning(new RooUniformBinning(minValue,maxValue,100))
_binning(new RooUniformBinning(minValue,maxValue))
{
_fast = true ;

Expand Down Expand Up @@ -157,7 +157,7 @@ RooRealVar::RooRealVar(const char *name, const char *title,
double value, double minValue, double maxValue,
const char *unit) :
RooAbsRealLValue(name, title, unit), _error(-1), _asymErrLo(1), _asymErrHi(-1),
_binning(new RooUniformBinning(minValue,maxValue,100))
_binning(new RooUniformBinning(minValue,maxValue))
{
_fast = true ;
setRange(minValue,maxValue) ;
Expand Down Expand Up @@ -815,7 +815,7 @@ void RooRealVar::writeToStream(ostream &os, bool compact) const
os << " - +INF) ";
}

if (getBins() != 100) {
if (getBins() != 0) {
os << "B(" << getBins() << ") ";
}

Expand Down Expand Up @@ -868,7 +868,7 @@ void RooRealVar::printExtras(ostream& os) const
}
os << ") " ;

if (getBins()!=100) {
if (getBins()!=0) {
os << "B(" << getBins() << ") " ;
}

Expand Down
2 changes: 1 addition & 1 deletion roofit/roofitcore/src/RooUniformBinning.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void RooUniformBinning::setRange(double xlo, double xhi)

_xlo = xlo ;
_xhi = xhi ;
_binw = (xhi-xlo)/_nbins ;
_binw = _nbins>0 ? (xhi-xlo)/_nbins : 0.0 ;

// Delete any out-of-date boundary arrays at this point
_array.clear();
Expand Down
2 changes: 2 additions & 0 deletions roofit/roofitcore/test/stressRooFit_tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -1722,6 +1722,8 @@ class TestBasic306 : public RooUnitTest {

// Plot decay_gm(dt|dterr) at various values of dterr
RooPlot *frame = dt.frame(Title("Slices of decay(dt|dterr) at various dterr"));
// Sample dterr in 100 bins to select the slice values below with setBin()
dterr.setBins(100);
for (int ibin = 0; ibin < 100; ibin += 20) {
dterr.setBin(ibin);
decay_gm.plotOn(frame, Normalization(5.), Name(Form("curve_slice_%d", ibin)));
Expand Down
5 changes: 4 additions & 1 deletion roofit/roofitcore/test/testRooSimultaneous.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,10 @@ TEST(RooSimultaneous, PlotProjWDataExtended)
auto integrateLastCurve = [](RooPlot *plot) {
const double xmin = plot->getPlotVar()->getMin();
const double xmax = plot->getPlotVar()->getMax();
return plot->getCurve()->average(xmin, xmax) * plot->getPlotVar()->numBins();
// Multiply by the number of bins of the plot frame (not the plot
// variable, which no longer carries a default binning) to turn the
// average events-per-bin of the curve back into a total event count.
return plot->getCurve()->average(xmin, xmax) * plot->GetNbinsX();
};

constexpr double tol = 0.01; // tolerate 1 % sampling error
Expand Down
19 changes: 19 additions & 0 deletions roofit/roostats/src/AsymptoticCalculator.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,20 @@ RooAbsData * GenerateAsimovDataSinglePdf(const RooAbsPdf & pdf, const RooArgSet

RooArgList obsList(*obs);

// The Asimov data set is built by looping over the bins of the
// observables. A freshly-constructed RooRealVar now defaults to zero bins,
// so temporarily materialize the historical default binning for any
// observable that has no explicit binning set. The change is undone right
// after filling the bins so that the model observables are left untouched.
std::vector<RooRealVar *> obsWithDefaultBinning;
for (auto *arg : obsList) {
auto *rrv = dynamic_cast<RooRealVar *>(arg);
if (rrv && rrv->getBins() == 0) {
rrv->setBins(RooRealVar::DefaultNBins);
obsWithDefaultBinning.push_back(rrv);
}
}

// loop on observables and on the bins
if (printLevel >= 2) {
oocoutI(nullptr,Generation) << "Generating Asimov data for pdf " << pdf.GetName() << std::endl;
Expand All @@ -1122,6 +1136,11 @@ RooAbsData * GenerateAsimovDataSinglePdf(const RooAbsPdf & pdf, const RooArgSet
double binVolume = 1;
int nbins = 0;
FillBins(pdf, obsList, *asimovData, obsIndex, binVolume, nbins);

// restore the zero-bins default on observables that had no explicit binning
for (auto *rrv : obsWithDefaultBinning) {
rrv->setBins(0);
}
if (printLevel >= 2)
oocoutI(nullptr,Generation) << "filled from " << pdf.GetName() << " " << nbins << " nbins " << " volume is " << binVolume << std::endl;

Expand Down
2 changes: 2 additions & 0 deletions tutorials/roofit/roofit/rf306_condpereventerrors.C
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ void rf306_condpereventerrors()

// Plot decay_gm(dt|dterr) at various values of dterr
RooPlot *frame = dt.frame(Title("Slices of decay(dt|dterr) at various dterr"));
// Sample dterr in 100 bins to select the slice values below with setBin()
dterr.setBins(100);
for (Int_t ibin = 0; ibin < 100; ibin += 20) {
dterr.setBin(ibin);
decay_gm.plotOn(frame, Normalization(5.));
Expand Down
2 changes: 2 additions & 0 deletions tutorials/roofit/roofit/rf306_condpereventerrors.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@

# Plot decay_gm(dt|dterr) at various values of dterr
frame = dt.frame(Title="Slices of decay(dt|dterr) at various dterr")
# Sample dterr in 100 bins to select the slice values below with setBin()
dterr.setBins(100)
for ibin in range(0, 100, 20):
dterr.setBin(ibin)
decay_gm.plotOn(frame, Normalization=5.0)
Expand Down
Loading