Skip to content

Commit 7d204dc

Browse files
PhmonskiPhmonski
committed
[HS3] Round-trip RooConstVars via domains
1 parent ec5952f commit 7d204dc

5 files changed

Lines changed: 176 additions & 20 deletions

File tree

‎roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class RooJSONFactoryWSTool {
4141
static constexpr bool useListsInsteadOfDicts = true;
4242
static bool allowExportInvalidNames;
4343
static bool allowSanitizeNames;
44+
static bool importNoDomainParametersAsRooConstVars;
4445
static RooWorkspace sanitizeWS(const RooWorkspace &ws);
4546
static RooWorkspace cleanWS(const RooWorkspace &ws, bool onlyModelConfig = false);
4647

‎roofit/hs3/src/Domains.cxx‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,23 @@ void Domains::writeJSON(RooFit::Detail::JSONNode &node) const
9191
}
9292
}
9393

94+
bool Domains::hasVariable(const char *name) const
95+
{
96+
for (auto const &domain : _map) {
97+
if (domain.second.hasVariable(name)) {
98+
return true;
99+
}
100+
}
101+
return false;
102+
}
103+
94104
void Domains::ProductDomain::readVariable(RooRealVar const &var)
95105
{
96106
readVariable(var.GetName(), var.getMin(), var.getMax());
97107
}
98108

99109
void Domains::ProductDomain::readVariable(const char *name, double min, double max)
100110
{
101-
if (RooNumber::isInfinite(min) && RooNumber::isInfinite(max))
102-
return;
103-
104111
auto &elem = _map[name];
105112

106113
if (!RooNumber::isInfinite(min)) {
@@ -123,6 +130,12 @@ void Domains::ProductDomain::writeVariable(RooRealVar &var) const
123130
var.setMax(elem.max);
124131
}
125132
}
133+
134+
bool Domains::ProductDomain::hasVariable(const char *name) const
135+
{
136+
return _map.find(name) != _map.end();
137+
}
138+
126139
void Domains::ProductDomain::readJSON(RooFit::Detail::JSONNode const &node)
127140
{
128141
if (!node.has_child("type") || node["type"].val() != "product_domain") {
@@ -162,7 +175,7 @@ void Domains::ProductDomain::populate(RooWorkspace &ws) const
162175
{
163176
for (auto const &item : _map) {
164177
const auto &name = item.first;
165-
if (!ws.var(name)) {
178+
if (!ws.arg(name)) {
166179
const auto &elem = item.second;
167180
const double vMin = elem.hasMin ? elem.min : -RooNumber::infinity();
168181
const double vMax = elem.hasMax ? elem.max : RooNumber::infinity();
@@ -176,7 +189,9 @@ void Domains::ProductDomain::registerBinnings(const char *name, RooWorkspace &ws
176189
auto *var = ws.var(item.first);
177190
if (!var)
178191
continue;
179-
var->setRange(name, item.second.min, item.second.max);
192+
const double vMin = item.second.hasMin ? item.second.min : -RooNumber::infinity();
193+
const double vMax = item.second.hasMax ? item.second.max : RooNumber::infinity();
194+
var->setRange(name, vMin, vMax);
180195
}
181196
}
182197

‎roofit/hs3/src/Domains.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ class Domains {
4242
void readJSON(RooFit::Detail::JSONNode const &);
4343
void writeJSON(RooFit::Detail::JSONNode &) const;
4444

45+
bool hasVariable(const char *name) const;
46+
4547
void populate(RooWorkspace &ws) const;
4648

4749
class ProductDomain {
@@ -53,6 +55,8 @@ class Domains {
5355
void readJSON(RooFit::Detail::JSONNode const &);
5456
void writeJSON(RooFit::Detail::JSONNode &) const;
5557

58+
bool hasVariable(const char *name) const;
59+
5660
void populate(RooWorkspace &ws) const;
5761
void registerBinnings(const char *name, RooWorkspace &ws) const;
5862

‎roofit/hs3/src/RooJSONFactoryWSTool.cxx‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -519,9 +519,6 @@ void exportAttributes(const RooAbsArg *arg, JSONNode &rootnode)
519519
node = &RooJSONFactoryWSTool::getRooFitInternal(rootnode, "attributes").set_map()[arg->GetName()].set_map();
520520
};
521521

522-
// RooConstVars are not a thing in HS3, and also for RooFit they are not
523-
// that important: they are just constants. So we don't need to remember
524-
// any information about them.
525522
if (dynamic_cast<RooConstVar const *>(arg)) {
526523
return;
527524
}
@@ -986,6 +983,7 @@ bool RooJSONFactoryWSTool::isValidName(const std::string &str)
986983

987984
bool RooJSONFactoryWSTool::allowExportInvalidNames(true);
988985
bool RooJSONFactoryWSTool::allowSanitizeNames(true);
986+
bool RooJSONFactoryWSTool::importNoDomainParametersAsRooConstVars(true);
989987
bool RooJSONFactoryWSTool::testValidName(const std::string &name, bool forceError)
990988
{
991989
if (!RooJSONFactoryWSTool::isValidName(name)) {
@@ -1052,6 +1050,8 @@ RooAbsReal *RooJSONFactoryWSTool::requestImpl<RooAbsReal>(const std::string &obj
10521050
return pdf;
10531051
if (RooRealVar *var = requestImpl<RooRealVar>(objname))
10541052
return var;
1053+
if (RooAbsReal *retval = _workspace.function(objname))
1054+
return retval;
10551055
auto it = _functionsByName.find(objname);
10561056
if (it != _functionsByName.end()) {
10571057
this->importFunction(*it->second, true);
@@ -1794,22 +1794,20 @@ void RooJSONFactoryWSTool::importVariable(const JSONNode &p)
17941794
std::string name(RooJSONFactoryWSTool::name(p));
17951795
RooJSONFactoryWSTool::testValidName(name, true);
17961796

1797-
if (_workspace.var(name))
1797+
if (_workspace.arg(name))
17981798
return;
17991799
if (!p.is_map()) {
18001800
std::stringstream ss;
18011801
ss << "RooJSONFactoryWSTool() node '" << name << "' is not a map, skipping.";
18021802
oocoutE(nullptr, InputArguments) << ss.str() << std::endl;
18031803
return;
18041804
}
1805-
if (_attributesNode) {
1806-
if (auto *attrNode = _attributesNode->find(name)) {
1807-
// We should not create RooRealVar objects for RooConstVars!
1808-
if (attrNode->has_child("is_const_var") && (*attrNode)["is_const_var"].val_int() == 1) {
1809-
wsEmplace<RooConstVar>(name, p["value"].val_double());
1810-
return;
1811-
}
1805+
if (importNoDomainParametersAsRooConstVars && !_domains->hasVariable(name.c_str())) {
1806+
if (!p.has_child("value")) {
1807+
RooJSONFactoryWSTool::error("cannot instantiate RooConstVar '" + name + "' without \"value\"!");
18121808
}
1809+
wsEmplace<RooConstVar>(name, p["value"].val_double());
1810+
return;
18131811
}
18141812
configureVariable(*_domains, p, wsEmplace<RooRealVar>(name, 1.));
18151813
}
@@ -2273,16 +2271,16 @@ void RooJSONFactoryWSTool::importAllNodes(const JSONNode &n)
22732271
error(ss.str());
22742272
}
22752273

2274+
_rootnodeInput = &n;
2275+
2276+
_attributesNode = findRooFitInternal(*_rootnodeInput, "attributes");
2277+
22762278
_domains = std::make_unique<RooFit::JSONIO::Detail::Domains>();
22772279
if (auto domains = n.find("domains")) {
22782280
_domains->readJSON(*domains);
22792281
}
22802282
_domains->populate(_workspace);
22812283

2282-
_rootnodeInput = &n;
2283-
2284-
_attributesNode = findRooFitInternal(*_rootnodeInput, "attributes");
2285-
22862284
// Build name-keyed indices over the "functions" and "distributions"
22872285
// sequences. Without these, every cross-reference resolved during import
22882286
// (e.g. dependencies of a PiecewiseInterpolation, or factory-expression

‎roofit/hs3/test/testRooFitHS3.cxx‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,38 @@ std::string parameterStepWidthsNode(std::string const &json)
146146
return json.substr(begin, end - begin + 1);
147147
}
148148

149+
std::string defaultDomainAxesNode(std::string const &json)
150+
{
151+
const std::string key = "\"domains\":[";
152+
const auto domainsBegin = json.find(key);
153+
if (domainsBegin == std::string::npos) {
154+
return "";
155+
}
156+
const auto axesBegin = json.find("\"axes\":[", domainsBegin);
157+
if (axesBegin == std::string::npos) {
158+
return "";
159+
}
160+
const auto axesEnd = json.find("}]", axesBegin);
161+
if (axesEnd == std::string::npos) {
162+
return "";
163+
}
164+
return json.substr(axesBegin, axesEnd - axesBegin + 2);
165+
}
166+
167+
class ScopedNoDomainConstVarImportFlag {
168+
public:
169+
explicit ScopedNoDomainConstVarImportFlag(bool value)
170+
: _oldValue{RooJSONFactoryWSTool::importNoDomainParametersAsRooConstVars}
171+
{
172+
RooJSONFactoryWSTool::importNoDomainParametersAsRooConstVars = value;
173+
}
174+
175+
~ScopedNoDomainConstVarImportFlag() { RooJSONFactoryWSTool::importNoDomainParametersAsRooConstVars = _oldValue; }
176+
177+
private:
178+
bool _oldValue;
179+
};
180+
149181
} // namespace
150182

151183
// Test that the IO of attributes and string attributes works.
@@ -294,6 +326,7 @@ TEST(RooFitHS3, ParameterStepWidthsImportAfterDefaultSnapshot)
294326
})";
295327

296328
RooWorkspace ws{"workspace"};
329+
ScopedNoDomainConstVarImportFlag flagGuard{false};
297330
ASSERT_TRUE(RooJSONFactoryWSTool{ws}.importJSONfromString(json));
298331

299332
ASSERT_NE(ws.var("mu"), nullptr);
@@ -395,6 +428,111 @@ TEST(RooFitHS3, RooGaussian)
395428
EXPECT_EQ(status, 0);
396429
}
397430

431+
TEST(RooFitHS3, RooGaussianConstVarSigmaExport)
432+
{
433+
ScopedNoDomainConstVarImportFlag flagGuard{true};
434+
435+
RooRealVar x{"x", "x", 0.0, -10.0, 10.0};
436+
RooRealVar mean{"mean", "mean", 0.0};
437+
mean.setConstant(true);
438+
439+
RooConstVar sigmaConst{"sigma_const", "sigma_const", 1.0};
440+
RooGaussian gaussConst{"gauss_const", "gauss_const", x, mean, sigmaConst};
441+
442+
RooGaussian gaussLiteral{"gauss_literal", "gauss_literal", x, mean, RooFit::RooConst(2.0)};
443+
444+
RooRealVar sigmaReal{"sigma_real", "sigma_real", 1.0, 0.1, 10.0};
445+
sigmaReal.setConstant(true);
446+
RooGaussian gaussReal{"gauss_real", "gauss_real", x, mean, sigmaReal};
447+
448+
RooWorkspace ws;
449+
ws.import(gaussConst, RooFit::Silence());
450+
ws.import(gaussLiteral, RooFit::RecycleConflictNodes(), RooFit::Silence());
451+
ws.import(gaussReal, RooFit::RecycleConflictNodes(), RooFit::Silence());
452+
453+
const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString();
454+
const std::string domainAxes = defaultDomainAxesNode(json);
455+
ASSERT_FALSE(domainAxes.empty()) << json;
456+
457+
EXPECT_NE(json.find("\"sigma\":\"sigma_const\""), std::string::npos);
458+
EXPECT_NE(json.find("\"name\":\"sigma_const\""), std::string::npos);
459+
EXPECT_EQ(json.find("is_const_var"), std::string::npos);
460+
EXPECT_EQ(json.find("\"sigma\":1.0"), std::string::npos);
461+
EXPECT_NE(json.find("\"sigma\":2.0"), std::string::npos);
462+
EXPECT_EQ(domainAxes.find("\"name\":\"sigma_const\""), std::string::npos) << domainAxes;
463+
464+
EXPECT_NE(json.find("\"sigma\":\"sigma_real\""), std::string::npos);
465+
EXPECT_NE(json.find("\"name\":\"sigma_real\""), std::string::npos);
466+
EXPECT_NE(domainAxes.find("\"name\":\"sigma_real\""), std::string::npos) << domainAxes;
467+
468+
// The unbounded constant RooRealVar is still a RooRealVar, so it gets an
469+
// empty domain axis that distinguishes it from a RooConstVar.
470+
EXPECT_NE(domainAxes.find("\"name\":\"mean\""), std::string::npos) << domainAxes;
471+
EXPECT_EQ(domainAxes.find("\"name\":\"mean\",\"min\""), std::string::npos) << domainAxes;
472+
EXPECT_EQ(domainAxes.find("\"name\":\"mean\",\"max\""), std::string::npos) << domainAxes;
473+
474+
RooWorkspace imported;
475+
ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json));
476+
EXPECT_NE(dynamic_cast<RooConstVar *>(imported.obj("sigma_const")), nullptr);
477+
EXPECT_EQ(imported.var("sigma_const"), nullptr);
478+
EXPECT_NE(dynamic_cast<RooRealVar *>(imported.obj("sigma_real")), nullptr);
479+
EXPECT_NE(dynamic_cast<RooRealVar *>(imported.obj("mean")), nullptr);
480+
481+
const std::string roundTripJson = RooJSONFactoryWSTool{imported}.exportJSONtoString();
482+
const std::string roundTripDomainAxes = defaultDomainAxesNode(roundTripJson);
483+
EXPECT_NE(roundTripJson.find("\"sigma\":\"sigma_const\""), std::string::npos);
484+
EXPECT_EQ(roundTripJson.find("is_const_var"), std::string::npos);
485+
EXPECT_EQ(roundTripDomainAxes.find("\"name\":\"sigma_const\""), std::string::npos) << roundTripDomainAxes;
486+
487+
const std::string legacyJson = R"({
488+
"metadata":{"hs3_version":"0.2"},
489+
"parameter_points":[{"name":"default_values","parameters":[
490+
{"name":"x","value":0.0},
491+
{"name":"mean","value":0.0},
492+
{"name":"sigma_const","value":1.0,"const":true}
493+
]}],
494+
"distributions":[{"name":"gauss","type":"gaussian_dist","x":"x","mean":"mean","sigma":"sigma_const"}]
495+
})";
496+
RooWorkspace legacyImport;
497+
{
498+
ScopedNoDomainConstVarImportFlag legacyFlagGuard{false};
499+
ASSERT_TRUE(RooJSONFactoryWSTool{legacyImport}.importJSONfromString(legacyJson));
500+
}
501+
EXPECT_NE(dynamic_cast<RooRealVar *>(legacyImport.obj("sigma_const")), nullptr);
502+
EXPECT_EQ(dynamic_cast<RooConstVar *>(legacyImport.obj("sigma_const")), nullptr);
503+
}
504+
505+
TEST(RooFitHS3, RooConstVarCollectionProxyExport)
506+
{
507+
ScopedNoDomainConstVarImportFlag flagGuard{true};
508+
509+
RooRealVar x{"x", "x", 0.0, -10.0, 10.0};
510+
RooRealVar mean1{"mean1", "mean1", -1.0};
511+
RooRealVar mean2{"mean2", "mean2", 1.0};
512+
RooRealVar sigma{"sigma", "sigma", 1.0, 0.1, 10.0};
513+
514+
RooGaussian g1{"g1", "g1", x, mean1, sigma};
515+
RooGaussian g2{"g2", "g2", x, mean2, sigma};
516+
RooConstVar frac{"frac_const", "frac_const", 0.25};
517+
RooAddPdf model{"model", "model", RooArgList{g1, g2}, RooArgList{frac}};
518+
519+
RooWorkspace ws;
520+
ws.import(model, RooFit::Silence());
521+
522+
const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString();
523+
const std::string domainAxes = defaultDomainAxesNode(json);
524+
ASSERT_FALSE(domainAxes.empty()) << json;
525+
EXPECT_NE(json.find("\"coefficients\":[\"frac_const\"]"), std::string::npos);
526+
EXPECT_NE(json.find("\"name\":\"frac_const\""), std::string::npos);
527+
EXPECT_EQ(json.find("is_const_var"), std::string::npos);
528+
EXPECT_EQ(domainAxes.find("\"name\":\"frac_const\""), std::string::npos) << domainAxes;
529+
530+
RooWorkspace imported;
531+
ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json));
532+
EXPECT_NE(dynamic_cast<RooConstVar *>(imported.obj("frac_const")), nullptr);
533+
EXPECT_EQ(imported.var("frac_const"), nullptr);
534+
}
535+
398536
TEST(RooFitHS3, RooBernstein)
399537
{
400538
int status = validate({"RooBernstein::bernstein(x[0, 10], { a[1], 3, b[5, 0, 20] })"});

0 commit comments

Comments
 (0)