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.

Sign up for GitHub

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

Jump to bottom

Trying to differentiate an operator in reverse mode fails. #917

Open
MihailMihov opened this issue May 31, 2024 · 2 comments · May be fixed by #984
Open

Trying to differentiate an operator in reverse mode fails. #917

MihailMihov opened this issue May 31, 2024 · 2 comments · May be fixed by #984
Assignees
@PetroZarytskyi
Milestone
v1.9

Comments

@MihailMihov
Copy link
Collaborator

MihailMihov commented May 31, 2024 •
edited by vgvassilev
Loading

Test:

#include "clad/Differentiator/Differentiator.h"

class SimpleFunctions1 {
public:
  SimpleFunctions1() noexcept : x(0), y(0) {}
  SimpleFunctions1(double p_x, double p_y) noexcept : x(p_x), y(p_y) {}
  double x;
  double y;
  double mem_fn_1(double i, double j) { return (x + y) * i + i * j * j; }
  SimpleFunctions1 operator+(const SimpleFunctions1& other) const {
    return SimpleFunctions1(x + other.x, y + other.y);
  }
};

double fn_s1_operator(double i, double j) {
  SimpleFunctions1 obj1(2, 3);
  SimpleFunctions1 obj2(3, 5);
  return (obj1 + obj2).mem_fn_1(i, j);
}

int main() {
  auto d_fn_s1_operator = clad::gradient(&fn_s1_operator);
}

Clang invocation:

/home/mihail/dev/llvm-18.1.5/build/bin/clang-18 -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all -dumpdir NonDifferentiable.out- -disable-free -clear-ast-before-backend -main-file-name NonDifferentiable.C -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu x86-64 -tune-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/home/mihail/dev/clad/build -fcoverage-compilation-dir=/home/mihail/dev/clad/build -resource-dir /home/mihail/dev/llvm-18.1.5/build/lib/clang/18 -I ../include -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13 -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/x86_64-pc-linux-gnu -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/13/include/g++-v13/backward -internal-isystem /home/mihail/dev/llvm-18.1.5/build/lib/clang/18/include -internal-isystem /usr/local/include -internal-isystem /usr/lib/gcc/x86_64-pc-linux-gnu/13/../../../../x86_64-pc-linux-gnu/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -ferror-limit 19 -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcxx-exceptions -fexceptions -fcolor-diagnostics -add-plugin clad -load /home/mihail/dev/clad/build/./lib/clad.so -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /tmp/NonDifferentiable-0608af.o -x c++ NonDifferentiable.C

Error:

NonDifferentiable.C:18:16: error: too few arguments to function call, expected 4, have 3
   15 | double fn_s1_operator(double i, double j) {
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   16 |   SimpleFunctions1 obj1(2, 3);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   17 |   SimpleFunctions1 obj2(3, 5);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   18 |   return (obj1 + obj2).mem_fn_1(i, j);
      |   ~~~~~~~~~~~~~^

The operator_plus_pullback that Clad is generating is:

void operator_plus_pullback(const SimpleFunctions1 &other, SimpleFunctions1 _d_y, SimpleFunctions1 *_d_this, SimpleFunctions1 *_d_other) const;

I don't understand what the error here is, so I'm not sure how to go about fixing this, but I discovered this when creating tests for [[clad::non_differentiable]] in reverse mode and this error is now blocking that PR.
@MihailMihov MihailMihov mentioned this issue Jun 1, 2024
Merged
@vgvassilev
Copy link
Owner

Don't we need to just pass the 4 arguments instead of 3?

@vgvassilev vgvassilev added this to the v1.6 milestone Jun 3, 2024
@gojakuch
Copy link
Collaborator

gojakuch commented Jun 22, 2024 •
edited
Loading

from my limited experience of working with operator differentiation in lambda functions, this error typically means that the constructed list of arguments for the pullback is ill-formed. the pullbacks normally accept a pointer to the derivative object of the class. I assume this may not be the case for non-differentiable types, which may result into this mismatch (if the VisitCallExpr method doesn't realise that we don't have to pass any pointers to the derivative objects to the pullback, it's gonna add this argument to the list). from what I also understand, clang does not count the first invisible argument, which is the object that calls the method itself, so in the AST it probably expects 4 args and receives 5 actually (if I remember correctly). not sure if it's exactly the case, but maybe that helps. it's only my assumption though and maybe you've already checked all of this and realised the issue

@vgvassilev vgvassilev modified the milestones: v1.6, v1.7 Jul 6, 2024
MihailMihov added a commit to MihailMihov/clad that referenced this issue Jul 17, 2024
@MihailMihov
Fix temporary expressions in reverse mode
c3df195 
fixes vgvassilev#917
MihailMihov added a commit to MihailMihov/clad that referenced this issue Jul 17, 2024
@MihailMihov
Fix temporary expressions in reverse mode
41a920e 
fixes vgvassilev#917
@MihailMihov MihailMihov linked a pull request Jul 17, 2024 that will close this issue
Draft
@vgvassilev vgvassilev modified the milestones: v1.7, v1.8 Aug 4, 2024
@vgvassilev vgvassilev modified the milestones: v1.8, v1.9 Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@PetroZarytskyi PetroZarytskyi

Labels
None yet
Projects
None yet
Milestone
v1.9
Development

Successfully merging a pull request may close this issue.

Fix temporary expressions in reverse mode MihailMihov/clad
4 participants
@vgvassilev @MihailMihov @gojakuch @PetroZarytskyi