Skip to content
Merged
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
88 changes: 88 additions & 0 deletions roofit/roofitcore/src/RooEvaluatorWrapper.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,91 @@ Wraps a RooFit::Evaluator that evaluates a RooAbsReal back into a RooAbsReal.

#include <fstream>

namespace {

// Throws an exception if any value in `span` is outside of the (default,
// unnamed) range of `var`. The `obsName` is used only for the error message,
// because it might differ from `var.GetName()` (e.g. the per-channel prefix
// that RooSimultaneous adds is stripped for readability).
void checkObservableSpanInRange(RooRealVar const &var, std::string const &obsName, std::string const &datasetName,
std::span<const double> span)
{
for (double val : span) {
if (!var.inRange(val, nullptr)) {
const double lo = var.getMin();
const double hi = var.getMax();
std::stringstream errMsg;
errMsg << "RooAbsPdf::fitTo/createNLL: cannot evaluate the likelihood because dataset \"" << datasetName
<< "\" has an entry for observable \"" << obsName << "\" with value " << val
<< ", which is outside of its range [" << lo << ", " << hi << "]. The probability density is "
<< "normalized over exactly that range, so events outside of it would silently bias the fit. If "
<< "you want to fit only a subset of the data, define a named range and use it in the fit, for example:\n"
<< " " << obsName << ".setRange(\"fitRange\", " << lo << ", " << hi << ");\n"
<< " pdf.fitTo(data, RooFit::Range(\"fitRange\"));\n"
<< "This way, only the events inside \"fitRange\" enter the likelihood, consistent with how the "
<< "pdf is normalized.";
oocoutE(nullptr, InputArguments) << errMsg.str() << std::endl;
throw std::runtime_error(errMsg.str());
}
}
}

// Validates that no dataset entry lies outside of the range of the
// corresponding observable, for every real-valued observable of `pdf`. This
// check is skipped when a range name was explicitly given to the fit,
// because in that case out-of-range events are intentionally and
// consistently dropped by RooFit::BatchModeDataHelpers::getDataSpans().
void validateObservableRanges(RooAbsPdf const *pdf, RooAbsData const &data,
std::map<RooFit::Detail::DataKey, std::span<const double>> const &dataSpans)
{
if (!pdf)
return;

if (auto const *simPdf = dynamic_cast<RooSimultaneous const *>(pdf)) {
// The per-channel pdfs coming out of RooSimultaneous::compileForNormSet()
// have their observables cloned and renamed with a "_<channel>_" prefix
// (and tagged with the "__obs__" attribute), so that the shared data map
// can hold independent columns for each channel. We look those up the
// same way, and strip the prefix again for a readable error message.
for (auto const &nameIdx : simPdf->indexCat()) {
RooAbsPdf *channelPdf = simPdf->getPdf(nameIdx.first);
if (!channelPdf)
continue;
const std::string prefix = "_" + nameIdx.first + "_";
std::unique_ptr<RooArgSet> vars{channelPdf->getVariables()};
std::unique_ptr<RooArgSet> obs{vars->selectByAttrib("__obs__", true)};
for (RooAbsArg *arg : *obs) {
auto *realVar = dynamic_cast<RooRealVar *>(arg);
if (!realVar)
continue;
auto it = dataSpans.find(RooFit::Detail::DataKey(realVar));
if (it == dataSpans.end())
continue;
std::string obsName = realVar->GetName();
if (obsName.rfind(prefix, 0) == 0) {
obsName = obsName.substr(prefix.size());
}
checkObservableSpanInRange(*realVar, obsName, data.GetName(), it->second);
}
}
return;
}

RooArgSet obs;
pdf->getObservables(data.get(), obs);
for (RooAbsArg *arg : obs) {
auto *realVar = dynamic_cast<RooRealVar *>(arg);
if (!realVar)
continue;
auto it = dataSpans.find(RooFit::Detail::DataKey(realVar));
if (it == dataSpans.end())
continue;
checkObservableSpanInRange(*realVar, realVar->GetName(), data.GetName(), it->second);
}
}

} // namespace

namespace RooFit::Experimental {

RooEvaluatorWrapper::RooEvaluatorWrapper(RooAbsReal &topNode, RooAbsData *data, bool useGPU,
Expand Down Expand Up @@ -669,6 +754,9 @@ bool RooEvaluatorWrapper::setData(RooAbsData &data, bool /*cloneData*/)
auto simPdf = dynamic_cast<RooSimultaneous const *>(_pdf);
_dataSpans = RooFit::BatchModeDataHelpers::getDataSpans(*_data, _rangeName, simPdf, skipZeroWeights,
_takeGlobalObservablesFromData, _vectorBuffers);
if (_rangeName.empty()) {
validateObservableRanges(_pdf, *_data, _dataSpans);
}
if (!isInitializing && _dataSpans.size() != oldSize) {
coutE(DataHandling) << errMsg << std::endl;
throw std::runtime_error(errMsg);
Expand Down
59 changes: 59 additions & 0 deletions roofit/roofitcore/test/testRooAbsPdf.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,65 @@ TEST_P(FitTest, PdfAsFunctionInFormulaVar)
EXPECT_FLOAT_EQ(nll->getVal(), ref);
}

// If an observable's range is shrunk after a dataset was already filled, the
// dataset still contains entries that are now outside of the range over which
// the pdf is normalized. Evaluating the likelihood anyway would silently bias
// the fit. The vectorizing evaluation backends must detect this and throw a
// descriptive error, while the correct workaround (a named range) keeps
// working for all backends. Covers GitHub issue #22740.
TEST_P(FitTest, OutOfRangeDataThrows)
{
using namespace RooFit;

RooWorkspace ws;
ws.factory("Gaussian::gauss(x[0, 5], mean[0.5, -10, 10], sigma[1.0, 0.1, 10.0])");

auto &x = *ws.var("x");
auto &mean = *ws.var("mean");
RooAbsPdf &gauss = *ws.pdf("gauss");

// Fill a dataset with entries at 1, 2 and 3 while the range is still [0, 5].
RooDataSet data{"data", "data", x};
for (double val : {1.0, 2.0, 3.0}) {
x.setVal(val);
for (int i = 0; i < 50; ++i) {
data.add(x);
}
}

// Shrink the range so that the entries at 3 are now out of range, while the
// dataset's internal clone of the observable still remembers [0, 5].
x.setMax(2.5);

const bool isLegacy = _evalBackend == EvalBackend::Legacy();

{
// Normalizing over [0, 2.5] while still evaluating the entries at 3 would
// bias the fit, so the vectorizing backends throw. The legacy backend is
// not affected by this check and keeps its historical behavior.
RooHelpers::HijackMessageStream hijack(RooFit::ERROR, RooFit::InputArguments);
auto doFit = [&]() {
std::unique_ptr<RooFitResult>{gauss.fitTo(data, _evalBackend, Save(), PrintLevel(-1))};
};
if (isLegacy) {
EXPECT_NO_THROW(doFit());
} else {
EXPECT_THROW(doFit(), std::runtime_error);
}
}

// Restricting the fit with a named range is the correct approach: the
// out-of-range entries at 3 are dropped consistently for both normalization
// and evaluation. This must work for every backend and recover a mean close
// to the mean of the clipped data (50 entries at 1 and 50 at 2).
mean.setVal(0.5);
x.setRange("fitRange", 0, 2.5);
std::unique_ptr<RooFitResult> result{gauss.fitTo(data, _evalBackend, Range("fitRange"), Save(), PrintLevel(-1))};
ASSERT_NE(result, nullptr);
EXPECT_EQ(result->status(), 0);
EXPECT_NEAR(mean.getVal(), 1.5, 0.2);
}

// Verifies that a server pdf gets correctly reevaluated when the normalization
// set is changed.
TEST(RooAbsPdf, NormSetChange)
Expand Down
Loading