Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support of custom _forw functions #1037

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

infinite-void-16
Copy link
Contributor

@infinite-void-16 infinite-void-16 commented Aug 10, 2024

This commit adds support for custom (user-provided) _forw functions.
A _forw function, if available, is called in place of the actual
function.

For example, if the primal code contains:

someFn(u, v, w);

and user has defined a custom _forw function for someFn as follows:

namespace clad {
  namespace custom_derivatives {
    void someFn_forw(double u, double v, double w, double *d_u,
      double *d_v, double *dw) {
      // ...
      // ...
    }
  }
}

Then clad will generate the derivative function as follows:

// forward-pass
clad::custom_derivatives::someFn_forw(u, v, w, d_u, d_v, d_w);
// ...

// reverse-pass; no change in reverse-pass
someFn_pullback(u, v, w, d_u, d_v, d_w);
// ...

But more importantly, why do we need such a functionality? Two reasons:

  • Supporting reference/pointer return types in the reverse-mode. This
    has been discussed at great length here:
    Add initial support for diff of ref return types in rev mode #425 (Add initial support for diff of ref return types in rev mode #425)

  • Supporting types whose elements grows dynamically, such as
    std::vector and std::map. The issue is that we correctly
    need to update the size/property of the adjoint variable when a
    function call updates the size/property of the corresponding primal
    variable. For example: a call to vec.push_back(...) should update
    the size of _d_vec as well. However, the actual function call does
    not modify the adjoint variable in any way. Here comes _forw functions
    to the rescue. _forw functions makes it possible to adjust the adjoint
    variable size/properties along with executing the actual function call.

Please note that _forw function signature takes adjoint variables as
arguments and return clad::ValueAndAdjoint<U, V> to support the
reference/pointer return type.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Expr* customForwPassCE =
m_Builder.BuildCallToCustomDerivativeOrNumericalDiff(
forwPassFnName, args, getCurrentScope(),
const_cast<DeclContext*>(FD->getDeclContext()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

            const_cast<DeclContext*>(FD->getDeclContext()));
            ^

Copy link

codecov bot commented Aug 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.12%. Comparing base (3f5bfd0) to head (6ede83c).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1037      +/-   ##
==========================================
+ Coverage   94.09%   94.12%   +0.02%     
==========================================
  Files          55       55              
  Lines        8250     8275      +25     
==========================================
+ Hits         7763     7789      +26     
+ Misses        487      486       -1     
Files Coverage Δ
include/clad/Differentiator/BuiltinDerivatives.h 100.00% <ø> (ø)
include/clad/Differentiator/CladUtils.h 100.00% <ø> (ø)
include/clad/Differentiator/Differentiator.h 83.33% <ø> (ø)
include/clad/Differentiator/ReverseModeVisitor.h 97.97% <ø> (ø)
lib/Differentiator/CladUtils.cpp 93.89% <100.00%> (+0.04%) ⬆️
lib/Differentiator/ReverseModeVisitor.cpp 97.77% <100.00%> (+0.02%) ⬆️

... and 2 files with indirect coverage changes

Files Coverage Δ
include/clad/Differentiator/BuiltinDerivatives.h 100.00% <ø> (ø)
include/clad/Differentiator/CladUtils.h 100.00% <ø> (ø)
include/clad/Differentiator/Differentiator.h 83.33% <ø> (ø)
include/clad/Differentiator/ReverseModeVisitor.h 97.97% <ø> (ø)
lib/Differentiator/CladUtils.cpp 93.89% <100.00%> (+0.04%) ⬆️
lib/Differentiator/ReverseModeVisitor.cpp 97.77% <100.00%> (+0.02%) ⬆️

... and 2 files with indirect coverage changes

@infinite-void-16 infinite-void-16 force-pushed the CustomForw branch 2 times, most recently from 58129cd to a14f6aa Compare August 10, 2024 20:57
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

m_Builder.BuildCallToCustomDerivativeOrNumericalDiff(
forwPassFnName, args, getCurrentScope(),
const_cast<DeclContext*>(FD->getDeclContext()));
return customForwPassCE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

            const_cast<DeclContext*>(FD->getDeclContext()));
            ^

@infinite-void-16 infinite-void-16 marked this pull request as ready for review August 10, 2024 21:24
@infinite-void-16 infinite-void-16 changed the title Add support of custom _forw functions Add support of custom _forw functions Aug 10, 2024
Copy link
Owner

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! @PetroZarytskyi, can you take a look at the change in the TBR part?

Copy link
Collaborator

@gojakuch gojakuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! this should help a lot. @parth-07 is this consistent with what you'd thought about custom constructor support in the reverse mode? if so, I think we can do smth similar for them, I suppose

include/clad/Differentiator/STLBuiltins.h Outdated Show resolved Hide resolved
test/Gradient/UserDefinedTypes.C Outdated Show resolved Hide resolved
Copy link
Collaborator

@gojakuch gojakuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This commit adds support for custom (user-provided) `_forw` functions.
A `_forw` function, if available, is called in place of the actual
function.

For example, if the primal code contains:

```cpp
someFn(u, v, w);
```

and user has defined a custom `_reverse_forw` function for `someFn` as follows:

```cpp
namespace clad {
  namespace custom_derivatives {
    void someFn_reverse_forw(double u, double v, double w, double *d_u,
      double *d_v, double *dw) {
      // ...
      // ...
    }
  }
}
```

Then clad will generate the derivative function as follows:

```cpp
// forward-pass
clad::custom_derivatives::someFn_reverse_forw(u, v, w, d_u, d_v, d_w);
// ...

// reverse-pass; no change in reverse-pass
someFn_pullback(u, v, w, d_u, d_v, d_w);
// ...
```

But more importantly, why do we need such a functionality? Two reasons:

- Supporting reference/pointer return types in the reverse-mode. This
  has been discussed at great length here:
vgvassilev#425 (vgvassilev#425)

- Supporting types whose elements grows dynamically, such as
  `std::vector` and `std::map`. The issue is that we correctly
  need to update the size/property of the adjoint variable when a
  function call updates the size/property of the corresponding primal
  variable. For example: a call to `vec.push_back(...)` should update
  the size of `_d_vec` as well. However, the actual function call does
  not modify the adjoint variable in any way. Here comes `_forw` functions
  to the rescue. `_forw` functions makes it possible to adjust the adjoint
  variable size/properties along with executing the actual function call.

Please note that `_reverse_forw` function signature takes adjoint variables as
arguments and return `clad::ValueAndAdjoint<U, V>` to support the
reference/pointer return type.
@vgvassilev vgvassilev merged commit cfea783 into vgvassilev:master Aug 20, 2024
89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants