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

Derived variables should never be const in Reverse Mode AD #430

Closed
parth-07 opened this issue Apr 21, 2022 · 5 comments · Fixed by #517
Closed

Derived variables should never be const in Reverse Mode AD #430

parth-07 opened this issue Apr 21, 2022 · 5 comments · Fixed by #517

Comments

@parth-07
Copy link
Collaborator

A reproducible example:

double fn(double i, double j) {
  const double a = i;
  double res = a;
  return res;
}

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

Compiling this program gives the following error:

error: cannot assign to variable '_d_a' with const-qualified type 'const double'
issue.cpp:3:8: note: variable '_d_a' declared const here
double fn(double i, double j) {
       ^~
1 error generated.

The root cause of the error is that we are creating the derived variable of a const type -- because the corresponding primal variable is of const type. But even if the primal variable is of const type and cannot be modified in the primal function, the derived variable can still be modified and thus shouldn't be const.

@arora-vidushi
Copy link
Contributor

Hello,

I am a Master's in Computer Science from the University of Stuttgart, Germany, and currently am attending the Simulation Software Engineering (SSE) course. https://simulation-software-engineering.github.io/homepage/

For this course, we students were asked to select an open-source project in the simulation universe (this includes equation solvers, meshing..) and make a small contribution. This is to become acquainted with such open-source software development tool and their practices.

For this, I am interested in Clad and am now looking for something to work on. There are no clear expectations as to what this contribution should be. It can be one of the following, a new feature, a tutorial, some documentation or a bug fix.

I am new to this field of simulation software but eager to learn and would really like to contribute something useful.

I have done some research into this issue:
Clad performs reverse-mode differentiation using ReverseModeVisitor class and differentiates declaration statements and creates derivative variables in the function. Here this function "For each variable declaration v, create another declaration _d_v to store derivatives for potential reassignments."
I think I need to modify this function to create a derivative variable of the correct type.

So is it possible if I work on this "good first issue"? Also, maybe you can give me some feedback and pointers here on the issue and how can I contribute.

Thanks!

@vgvassilev
Copy link
Owner

Hello,

I am a Master's in Computer Science from the University of Stuttgart, Germany, and currently am attending the Simulation Software Engineering (SSE) course. https://simulation-software-engineering.github.io/homepage/

For this course, we students were asked to select an open-source project in the simulation universe (this includes equation solvers, meshing..) and make a small contribution. This is to become acquainted with such open-source software development tool and their practices.

Awesome. Welcome!

For this, I am interested in Clad and am now looking for something to work on. There are no clear expectations as to what this contribution should be. It can be one of the following, a new feature, a tutorial, some documentation or a bug fix.

I am new to this field of simulation software but eager to learn and would really like to contribute something useful.

This issue is a good start. I opened another issue which is important for us but a good start: #510. I would be happy to chat if necessary about other potential projects. I believe you can find my email easily :)

I have done some research into this issue: Clad performs reverse-mode differentiation using ReverseModeVisitor class and differentiates declaration statements and creates derivative variables in the function. Here this function "For each variable declaration v, create another declaration _d_v to store derivatives for potential reassignments." I think I need to modify this function to create a derivative variable of the correct type.

So is it possible if I work on this "good first issue"? Also, maybe you can give me some feedback and pointers here on the issue and how can I contribute.

That would be awesome! If you do development with Clad the dev documentation might be useful. We also have weekly meetings on Wed where we can discuss more. See https://compiler-research.org/meetings/

@arora-vidushi
Copy link
Contributor

arora-vidushi commented Nov 27, 2022

Hello Vassil,

Thanks for the head start.
I am happy to also look into the issue #510.
Meanwhile, I would start working on this issue.

Can you please tell me if there are any contribution guidelines that I should follow?

Thanks,
Vidushi

@vgvassilev
Copy link
Owner

Hi @arora-vidushi, no further contribution guidelines at that point besides what I have linked in the previous reply. Let me know if you need any further help. I use skype, slack and discord. Please contact me via email to share my handles.

@arora-vidushi
Copy link
Contributor

The above-given example gives no error and the issue seems to have been fixed already for constant Derived variables. However, on testing different case scenarios, I came across the problem with constant derived reference variables.

For example:

#include "clad/Differentiator/Differentiator.h"

double fn(double i, double j) {
  double a = i;
  const double& ar = a;
  double res = ar;
  return res;
}

int main() {
  auto fn_grad = clad::gradient(fn);
  double d_i, d_j;
  fn_grad.execute(3, 5, &d_i, &d_j);
}

This program compiles without error but the output file gives a segmentation fault:

[1]    31889 segmentation fault  ./clad/demos/ConstIssue.out

By using the "-fdump-derived-fn" flag, the problematic derivative function was found as follow:

void fn_grad(double i, double j, clad::array_ref<double> _d_i, clad::array_ref<double> _d_j) {
    double _d_a = 0;
    double *_d_ar = 0;
    double _d_res = 0;
    double a = i;
    const double &ar = a;
    double res = ar;
    double fn_return = res;
    goto _label0;
  _label0:
    _d_res += 1;
    *_d_ar += _d_res;
    * _d_i += _d_a;
}

I am also looking into why this issue occurred. Also, I would like to know if I should continue working on it as the issue #430 itself or whether I should create a separate issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants