clad icon indicating copy to clipboard operation
clad copied to clipboard

Don't create adjoints for integral type variables

Open PetroZarytskyi opened this issue 10 months ago • 3 comments

This PR removes adjoints for integral types in the reverse mode. This is done because integers are not differentiable in the traditional mathematical sense. Such behavior is consistent with other AD tools like Tapenade and Enzyme. Let's consider an example to see the difference in behavior

double f(double x) {
    int n = x;
    return n;
}

This is essentially a floor function so mathematically the derivative should be 0. However, Clad master produces 1. Let's compare the derivatives on master and in this PR. master:

void f_grad(double x, double *_d_x) {
    int _d_n = 0;
    int n = x;
    goto _label0;
  _label0:
    _d_n += 1;
    *_d_x += _d_n;
}

this PR:

void f_grad(double x, double *_d_x) {
    int n = x;
    goto _label0;
  _label0:
    ;
}

PetroZarytskyi avatar Apr 15 '24 11:04 PetroZarytskyi

Codecov Report

Attention: Patch coverage is 97.36842% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 94.58%. Comparing base (d1fec23) to head (63d07c5).

:exclamation: Current head 63d07c5 differs from pull request most recent head 6183d10

Please upload reports for the commit 6183d10 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
- Coverage   94.82%   94.58%   -0.25%     
==========================================
  Files          52       51       -1     
  Lines        7708     7678      -30     
==========================================
- Hits         7309     7262      -47     
- Misses        399      416      +17     
Files Coverage Δ
...clude/clad/Differentiator/BaseForwardModeVisitor.h 100.00% <ø> (ø)
include/clad/Differentiator/CladUtils.h 100.00% <ø> (ø)
include/clad/Differentiator/DerivativeBuilder.h 100.00% <ø> (ø)
include/clad/Differentiator/Differentiator.h 100.00% <ø> (ø)
include/clad/Differentiator/ReverseModeVisitor.h 97.77% <100.00%> (-0.10%) :arrow_down:
include/clad/Differentiator/VisitorBase.h 100.00% <ø> (ø)
lib/Differentiator/CladUtils.cpp 93.58% <100.00%> (+0.81%) :arrow_up:
lib/Differentiator/DerivativeBuilder.cpp 100.00% <ø> (+0.99%) :arrow_up:
lib/Differentiator/ErrorEstimator.cpp 97.09% <100.00%> (-2.07%) :arrow_down:
lib/Differentiator/BaseForwardModeVisitor.cpp 98.71% <94.73%> (-0.20%) :arrow_down:
... and 2 more

... and 10 files with indirect coverage changes

Files Coverage Δ
...clude/clad/Differentiator/BaseForwardModeVisitor.h 100.00% <ø> (ø)
include/clad/Differentiator/CladUtils.h 100.00% <ø> (ø)
include/clad/Differentiator/DerivativeBuilder.h 100.00% <ø> (ø)
include/clad/Differentiator/Differentiator.h 100.00% <ø> (ø)
include/clad/Differentiator/ReverseModeVisitor.h 97.77% <100.00%> (-0.10%) :arrow_down:
include/clad/Differentiator/VisitorBase.h 100.00% <ø> (ø)
lib/Differentiator/CladUtils.cpp 93.58% <100.00%> (+0.81%) :arrow_up:
lib/Differentiator/DerivativeBuilder.cpp 100.00% <ø> (+0.99%) :arrow_up:
lib/Differentiator/ErrorEstimator.cpp 97.09% <100.00%> (-2.07%) :arrow_down:
lib/Differentiator/BaseForwardModeVisitor.cpp 98.71% <94.73%> (-0.20%) :arrow_down:
... and 2 more

... and 10 files with indirect coverage changes

codecov[bot] avatar Apr 30 '24 17:04 codecov[bot]

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar May 19 '24 07:05 github-actions[bot]

Since this is largely backward incompatible change, can you open this PR against the ver-2.0 branch?

vgvassilev avatar May 21 '24 07:05 vgvassilev