Enable some cases of functor calls in custom pushforwards

[gojakuch]

Previously, if a user wanted to provide a custom pushforward for a function that uses functors in it, it was impossible to use generated pushforwards for that functors' call operators. This PR aims to fix this for basic functors that don't have multiple call operator overloads.

Fixes: #1023

[gojakuch]

@vgvassilev this issue was kind of a blocker to general Kokkos support, so I'm fixing it in the way you've suggested and I hope this can be merged soon. I will also open another issue to generalise this approach for other operators and more complicated use-cases

[github-actions]

clang-tidy made some suggestions

[github-actions]

warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]

                     const_cast<DeclContext*>(FD->getDeclContext()), SS)
                     ^

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.11%. Comparing base (6cc83ee) to head (bba8991).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1038      +/-   ##
==========================================
+ Coverage   94.09%   94.11%   +0.02%     
==========================================
  Files          55       55              
  Lines        8248     8280      +32     
==========================================
+ Hits         7761     7793      +32     
  Misses        487      487              

Files	Coverage Δ
...clude/clad/Differentiator/BaseForwardModeVisitor.h	33.33% <ø> (ø)
lib/Differentiator/BaseForwardModeVisitor.cpp	98.57% <100.00%> (+0.03%)	⬆️

Files	Coverage Δ
...clude/clad/Differentiator/BaseForwardModeVisitor.h	33.33% <ø> (ø)
lib/Differentiator/BaseForwardModeVisitor.cpp	98.57% <100.00%> (+0.03%)	⬆️

[vgvassilev]

@parth-07, can you take a look?

[parth-07]

I believe we should not use clad-generated derivatives inside custom derivatives. This will make the custom-derivatives less portable because the compilation will fail for the cases where Clad has not generated the derivative.

Using clad-generated member function derivatives in custom derivatives can only work for template functions. This makes the functionality inconsistent. For example:

#include "clad/Differentiator/Differentiator.h"
#include <iostream>

struct Foo {
    double &y;
    Foo(double &y): y(y) {} 

    double operator()(double x) {
        y = 2*x;
        return x;
    }
};

namespace clad {
    namespace custom_derivatives {
        void use_functor_pushforward(double x, Foo &f, double d_x, Foo &d_f) {
            f.operator_call_pushforward(x, &d_f, d_x);
        }
    }
}

void use_functor(double x, Foo &f) {
    f(x);
}


double fn(double x) {
    Foo func = Foo(x);
    use_functor(x, func);
    return x;
}

int main() {
    auto fn_grad = clad::differentiate(fn);
}

Compilation of above fails with the error message:

FunctorCustomDerv.cpp:17:15: error: no member named 'operator_call_pushforward' in 'Foo'
            f.operator_call_pushforward(x, &d_f, d_x);

Things work in template function because of SFINAE rule and delayed instantiations of templates.

[gojakuch]

well, I need this template situation to be solved to support Kokkos in particular, and I think such a pattern would be relatively common when supporting other similar frameworks in Clad (or when the users try to do that). as I've mentioned, this is a blocker there. I agree that this PR only addresses things that occur when using templates and I'm okay with trying to generalise this. I don't really advocate for this approach specifically, but what I need here is that:

1. it should be possible to provide custom pushforwards for functions (like use_functor in this example) that the user can call with their data types without defining anything else themselves. so we need to be able to provide general template pushforwards. this way we could support those frameworks and not demand anything from the user, which is a requirement (the users should be able to use this in their existing applications at least theoretically and shouldn't modify the code of their classes to use them with Kokkos+Clad). the first thing I thought about was to just track the usage of undefined functions or methods with _pushforward in their name and then tell clad to put that into the differentiation plan properly, but I'm not sure how that can be implemented and if that doesn't introduce even more inconsistencies. Vassil has suggested that we can solve this in a way similar to how this PR works (just differentiate call operators in functors once they're used), so I did it like that. I should probably move these actions from the visitor to the planner though, that's my bad.

2. whatever the approach to solving this issue would be, I need it to work for both functors AND lambdas once we get the lambda support in Clad. what I mean is that the custom pushforward template function for the use_functor routine in this example should also work for lambdas. for lambdas, we'd differentiate their call methods as soon as the lambda is created anyway (as far as I imagine that right now), and we cannot rely on custom derivatives for the call methods at all, since there's kind of no way to provide these for lambdas from the user side, and I'm not sure how that would need to work even.

so I think what these two requirements imply is that the usage of generated pushforwards in custom derivatives is unavoidable in supporting some frameworks that operate with functors and lambdas like the Kokkos does (at least from what I see). but I'd be glad to use a more general or prettier approach here. can we hop on a call to resolve this maybe?

[parth-07]

If the goal is to differentiate use_functor, then can we provide custom derivative operator_call_pushforward(Functor *F, double &x, Functor *d_F, double d_x) and Clad can itself differentiate use_functor<Foo> instantiation? This is more general than providing a custom derivative for use_functor<Foo> and can be helpful if there are multiple functions using Foo functor.