Skip to content

Commit 28f39d5

Browse files
committed
[RF] Don't format infinities to large numbers anymore in codegen
By now, Clad should have learned how to deal with the `inf` infinities. The unit tests are tweaked the check this.
1 parent ba592ee commit 28f39d5

File tree

5 files changed

+28
-42
lines changed

5 files changed

+28
-42
lines changed

roofit/codegen/src/CodegenImpl.cxx

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ namespace Experimental {
6767

6868
namespace {
6969

70+
// Return a stringy-field version of the value, formatted to maximum precision.
71+
std::string doubleToString(double val)
72+
{
73+
std::stringstream ss;
74+
ss << std::setprecision(std::numeric_limits<double>::max_digits10) << val;
75+
return ss.str();
76+
}
77+
7078
std::string mathFunc(std::string const &name)
7179
{
7280
return "RooFit::Detail::MathFuncs::" + name;
@@ -249,8 +257,9 @@ void codegenImpl(RooMultiPdf &arg, CodegenContext &ctx)
249257

250258
// MathFunc call
251259

252-
if (numPdfs > 2) { // the value of this number should be discussed.Beyound a certain number of indices MathFunc call
253-
// becomes more efficient.
260+
// The value of this number should be discussed. Beyound a certain number of
261+
// indices MathFunc call becomes more efficient.
262+
if (numPdfs > 2) {
254263
ctx.addResult(&arg, ctx.buildCall(mathFunc("multipdf"), arg.indexCategory(), arg.getPdfList()));
255264

256265
std::cout << "MathFunc call used\n";
@@ -354,15 +363,7 @@ void codegenImpl(RooChebychev &arg, CodegenContext &ctx)
354363

355364
void codegenImpl(RooConstVar &arg, CodegenContext &ctx)
356365
{
357-
// Just return a stringy-field version of the const value.
358-
// Formats to the maximum precision.
359-
constexpr auto max_precision{std::numeric_limits<double>::max_digits10};
360-
std::stringstream ss;
361-
ss.precision(max_precision);
362-
// Just use toString to make sure we do not output 'inf'.
363-
// This is really ugly for large numbers...
364-
ss << std::fixed << RooNumber::toString(arg.getVal());
365-
ctx.addResult(&arg, ss.str());
366+
ctx.addResult(&arg, doubleToString(arg.getVal()));
366367
}
367368

368369
void codegenImpl(RooConstraintSum &arg, CodegenContext &ctx)
@@ -507,7 +508,7 @@ void codegenImpl(RooParamHistFunc &arg, CodegenContext &ctx)
507508
// be binned uniformly.
508509
double binV = arg.dataHist().binVolume(0);
509510
std::string weightArr = arg.dataHist().declWeightArrayForCodeSquash(ctx, false);
510-
result += " * *(" + weightArr + " + " + idx + ") * " + std::to_string(binV);
511+
result += " * *(" + weightArr + " + " + idx + ") * " + doubleToString(binV);
511512
}
512513
ctx.addResult(&arg, result);
513514
}
@@ -650,15 +651,7 @@ void codegenImpl(RooRealVar &arg, CodegenContext &ctx)
650651
if (!arg.isConstant()) {
651652
ctx.addResult(&arg, arg.GetName());
652653
}
653-
// Just return a formatted version of the const value.
654-
// Formats to the maximum precision.
655-
constexpr auto max_precision{std::numeric_limits<double>::max_digits10};
656-
std::stringstream ss;
657-
ss.precision(max_precision);
658-
// Just use toString to make sure we do not output 'inf'.
659-
// This is really ugly for large numbers...
660-
ss << std::fixed << RooNumber::toString(arg.getVal());
661-
ctx.addResult(&arg, ss.str());
654+
ctx.addResult(&arg, doubleToString(arg.getVal()));
662655
}
663656

664657
void codegenImpl(RooRecursiveFraction &arg, CodegenContext &ctx)
@@ -812,7 +805,7 @@ std::string rooHistIntegralTranslateImpl(int code, RooAbsArg const &arg, RooData
812805
<< std::endl;
813806
return "";
814807
}
815-
return std::to_string(dataHist.sum(histFuncMode));
808+
return doubleToString(dataHist.sum(histFuncMode));
816809
}
817810

818811
} // namespace
@@ -853,7 +846,7 @@ std::string codegenIntegralImpl(RooMultiVarGaussian &arg, int code, const char *
853846
throw std::runtime_error(errorMsg.str().c_str());
854847
}
855848

856-
return std::to_string(arg.analyticalIntegral(code, rangeName));
849+
return doubleToString(arg.analyticalIntegral(code, rangeName));
857850
}
858851

859852
std::string codegenIntegralImpl(RooPoisson &arg, int code, const char *rangeName, CodegenContext &ctx)
@@ -905,7 +898,7 @@ std::string codegenIntegralImpl(RooUniform &arg, int code, const char *rangeName
905898
{
906899
// The integral of a uniform distribution is static, so we can just hardcode
907900
// the result in a string.
908-
return std::to_string(arg.analyticalIntegral(code, rangeName));
901+
return doubleToString(arg.analyticalIntegral(code, rangeName));
909902
}
910903

911904
} // namespace Experimental

roofit/roofitcore/inc/RooFit/CodegenContext.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616

1717
#include <RooAbsCollection.h>
1818
#include <RooFit/EvalContext.h>
19-
#include <RooNumber.h>
2019

2120
#include <ROOT/RSpan.hxx>
2221

2322
#include <cstddef>
23+
#include <iomanip>
2424
#include <map>
2525
#include <sstream>
2626
#include <string>
@@ -150,7 +150,9 @@ class CodegenContext {
150150
template <class T, typename std::enable_if<std::is_floating_point<T>{}, bool>::type = true>
151151
std::string buildArg(T x)
152152
{
153-
return RooNumber::toString(x);
153+
std::stringstream ss;
154+
ss << std::setprecision(std::numeric_limits<double>::max_digits10) << x;
155+
return ss.str();
154156
}
155157

156158
// If input is integer, we want to print it into the code like one (i.e. avoid the unnecessary '.0000').

roofit/roofitcore/inc/RooNumber.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ class RooNumber {
3939
/// Get the absolute epsilon that is used by range checks in RooFit,
4040
/// e.g., in RooAbsRealLValue::inRange().
4141
inline static double rangeEpsAbs() { return staticRangeEpsAbs(); }
42-
static std::string toString(double x);
4342

4443
private:
4544
static double &staticRangeEpsRel();

roofit/roofitcore/src/RooNumber.cxx

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,6 @@ Provides numeric constants used in \ref Roofitmain.
2424

2525
#include <RooNumber.h>
2626

27-
/// @brief Returns an std::to_string compatible number (i.e. rounds infinities back to the nearest representable
28-
/// value). This function is primarily used in the code-squashing for AD and as such encodes infinities to double's
29-
/// maximum value. We do this because 1, std::to_string cannot handle infinities correctly on some platforms
30-
/// (e.g. 32 bit debian) and 2, Clad (the AD tool) cannot handle differentiating std::numeric_limits::infinity directly.
31-
std::string RooNumber::toString(double x)
32-
{
33-
int sign = isInfinite(x);
34-
double out = x;
35-
if (sign)
36-
out = sign == 1 ? std::numeric_limits<double>::max() : std::numeric_limits<double>::min();
37-
return std::to_string(out);
38-
}
39-
4027
double &RooNumber::staticRangeEpsRel()
4128
{
4229
static double epsRel = 0.0;

roofit/roofitcore/test/testRooFuncWrapper.cxx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,9 @@ TEST_P(FactoryTest, NLLFit)
219219
/// Initial minimization that was not based on any other tutorial/test.
220220
FactoryTestParams param1{"Gaussian",
221221
[](RooWorkspace &ws) {
222-
ws.factory("sum::mu_shifted(mu[0, -10, 10], shift[1.0, -10, 10])");
222+
constexpr double inf = std::numeric_limits<double>::infinity();
223+
ws.import(RooRealVar{"mu", "mu", -inf, inf});
224+
ws.factory("sum::mu_shifted(mu, shift[1.0, -10, 10])");
223225
ws.factory("prod::sigma_scaled(sigma[3.0, 0.01, 10], 1.5)");
224226
ws.factory("Gaussian::model(x[0, -10, 10], mu_shifted, sigma_scaled)");
225227

@@ -428,7 +430,10 @@ FactoryTestParams param9{"Poisson",
428430
// A RooPoisson where x is not rounded, like it is used in HistFactory
429431
FactoryTestParams param10{"PoissonNoRounding",
430432
[](RooWorkspace &ws) {
431-
ws.factory("Poisson::model(x[5, 0, 10], mu[5, 0, 10])");
433+
constexpr double inf = std::numeric_limits<double>::infinity();
434+
RooRealVar mu{"mu", "mu", 5., 0., inf};
435+
ws.import(mu);
436+
ws.factory("Poisson::model(x[5, 0, 10], mu)");
432437
auto poisson = static_cast<RooPoisson *>(ws.pdf("model"));
433438
poisson->setNoRounding(true);
434439
ws.defineSet("observables", "x");

0 commit comments

Comments
 (0)