Improvement: Avoid unexpected warnings about num-diff fallback

[guitargeek]

If I compile this example here:

// Compile with clang++ -std=c++11 -fplugin=/usr/lib/clad.so cladConfusingWarnings.cxx -o cladConfusingWarnings

#include "clad/Differentiator/Differentiator.h"

#include <iostream>

double func(double x)
{
   return x * std::tanh(1.0);
}

int main()
{
   double x = 2.0;
   std::cout << func(x) << std::endl;

   auto grad = clad::gradient(func, "x");

   double result = 0.0;
   grad.execute(x, &result);

   std::cout << result << std::endl;
}

Then I get a confusing warning:

warning: Falling back to numerical differentiation for 'tanh' since no suitable overload was found and clad could not derive it. To disable this feature, compile your programs with -DCLAD_NO_NUM_DIFF.

That is unexpected, because tanh(1.0) is a constant that doesn't need to be differentiated. Why is the fallback needed? I would expect no warning to show up in any case.

It would be good to suppress this warning, because it happens in the RooFit HistFactory likelihoods, where the LnGamma from ROOT math is used for constant expressions.

[guitargeek]

Hi! I think this issue should not be closed yet, because even though the PR fixes my simplified reproducer from the initial post, the warning still appears in the HistFactory context. I have also converted a HistFactory model to a standalone reproducer:

#include "clad/Differentiator/Differentiator.h"

#include <cmath>
#include <iostream>
#include <vector>

double func(double *params, double const *obs)
{
   double res = 0.0;
   double _signal_Hist_alphanominal_HistWeights[2] = {10.000000, 20.000000};
   double _background1_Hist_alphanominal_HistWeights[2] = {100.000000, 0.000000};
   double _background2_Hist_alphanominal_HistWeights[2] = {0.000000, 100.000000};
   double tmpVar4[] = {params[1], params[2]};
   double tmpVar6[] = {params[0], 1.0, 1.0};
   for (int loopIdx0 = 0; loopIdx0 < 2; loopIdx0++) {
      const double _signal_shapes = (_signal_Hist_alphanominal_HistWeights[loopIdx0]);
      const double _background1_shapes = (_background1_Hist_alphanominal_HistWeights[loopIdx0] * tmpVar4[loopIdx0]);
      const double _background2_shapes = (_background2_Hist_alphanominal_HistWeights[loopIdx0] * tmpVar4[loopIdx0]);
      double tmpVar0[] = {_signal_shapes, _background1_shapes, _background2_shapes};
      double tmpVar9 = 0;
      double tmpVar10 = 0;
      for (int i__model = 0; i__model < 3; i__model++) {
         tmpVar9 += tmpVar0[i__model] * tmpVar6[i__model];
      }
      res += -1 * (-tmpVar9 + obs[2 + loopIdx0] * std::log(tmpVar9) - std::lgamma(obs[2 + loopIdx0] + 1));
   }
   return res;
}

int main()
{
   std::vector<double> params{1.0, 0.0, 0.0};
   std::vector<double> obs{1.25, 1.75, 100.0, 100.0};
   std::cout << func(params.data(), obs.data()) << std::endl;

   auto grad = clad::gradient(func, "params");
}

The warning is still there:

warning: Falling back to numerical differentiation for 'lgamma' since no suitable overload was found and clad could not derive it. To disable this feature, compile your programs with -DCLAD_NO_NUM_DIFF.

[vaithak]

The crux of the issue seems to be that if some computation/expression is independent of the variables w.r.t which the differentiation is performed, these expressions can be treated as constant, implying that derivative expressions should not be generated for such cases. The initial PR of this issue helped in solving simple call expression cases where the arguments were literals and would have 0 derivatives.

For most cases, generating derivatives of such expressions will only affect the runtime complexity but will still run correctly. But in some cases, where generating such derivatives results in warnings/errors, this will cause issues. For the above example, the issue is that clad cannot properly generate pullback for lgamma function.

• A proper fix would include adding a custom pullback for lgamma function as a first step because this can also appear in other cases where its arguments can be the differentiation variable, where the pullback method is necessary.
  • @vgvassilev, do you think it makes sense to add the pullback of this in clad itself? Given that this is a somewhat less used function than those added in clad (like std::pow, std::min, etc.), would it make sense if the user adds a custom pullback for this manually? (if so, @guitargeek, you can use this for reference).
• Other than the above, if I am not wrong, I think @PetroZarytskyi is already looking into such cases where activity analysis can help in removing these unnecessary expressions.

[guitargeek]

Possibly related:

• https://github.com/vgvassilev/clad/issues/682

[PetroZarytskyi]

As Vaibhav explained above, this issue is currently blocked by #716.

[guitargeek]

Thanks for the update! From the RooFit point of view, this is still important because these warnings look scary to the users.