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

Incorrect results for comma expressions in forward mode AD #573

Closed
vaithak opened this issue Jun 17, 2023 · 0 comments · Fixed by #574
Closed

Incorrect results for comma expressions in forward mode AD #573

vaithak opened this issue Jun 17, 2023 · 0 comments · Fixed by #574

Comments

@vaithak
Copy link
Collaborator

vaithak commented Jun 17, 2023

Minimal example

double f(double x) {
   // returns (x+1)^2
   return (++x , x * x);
}

Theoretically expected derivative: 2*(x+1)

Generated code results in calculation of 2*x (i.e ++x is missing before returning the derivative):

double f_darg0(double x) {
    double _d_x = 1;
    return (_d_x * x + x * _d_x);
}

Similar, is the case with a slightly modified code for the same function:

double f(double x) {
   // returns (x+1)^2
   double temp = (++x , x * x);
   return temp;
}

Generated code again results in calculation of 2*x (this time ++x is misplaced)

double f_darg0(double x) {
    double _d_x = 1;
    double _d_temp = (_d_x * x + x * _d_x);
    double temp = (++x , x * x);
    return _d_temp;
}
@vaithak vaithak changed the title Issue with comma expressions in forward mode AD Incorrect results for comma expressions in forward mode AD Jun 17, 2023
vaithak added a commit to vaithak/clad that referenced this issue Jun 18, 2023
Earlier for an expression like `(E1, E2)`, the derivative code produced `(dE1, dE2)`,
thus a statement like `double temp = (++x, x*x)` produced:
```cpp
double _d_temp = (_d_x, _d_x * x + x * _d_x);
double temp = (++x, x*x);    <--- original cloned statement
```
This meant that the execution order is `dE1 -> dE2 -> E1 -> E2` (where -> means followed by).

But this doesn't seem right, as `dE2` can depend on `E1`; hence `E1` must be computed before `dE2`.
So, the execution order should be `dE1 -> E1 -> dE2 -> E2`.

This PR generates the derivative code as `(dE1, E1, dE2)`; the original statement is changed to just `E2`.
Note that the result of both expressions is still the same as earlier (i.e. the derivative code will still have the result of `dE2`)

So, the example statement of computing `(x+1)^2`, will produce:
```cpp
double _d_temp = (_d_x, ++x , (_d_x * x + x * _d_x));
double temp = x*x;
```

closes vgvassilev#573
vaithak added a commit to vaithak/clad that referenced this issue Jun 20, 2023
Earlier for an expression like `(E1, E2)`, the derivative code produced `(dE1, dE2)`,
thus a statement like `double temp = (++x, x*x)` produced:
```cpp
double _d_temp = (_d_x, _d_x * x + x * _d_x);
double temp = (++x, x*x);    <--- original cloned statement
```
This meant that the execution order is `dE1 -> dE2 -> E1 -> E2` (where -> means followed by).

But this doesn't seem right, as `dE2` can depend on `E1`; hence `E1` must be computed before `dE2`.
So, the execution order should be `dE1 -> E1 -> dE2 -> E2`.

This PR generates the derivative code as `(dE1, E1, dE2)`; the original statement is changed to just `E2`.
Note that the result of both expressions is still the same as earlier (i.e. the derivative code will still have the result of `dE2`)

So, the example statement of computing `(x+1)^2`, will produce:
```cpp
double _d_temp = (_d_x, ++x , (_d_x * x + x * _d_x));
double temp = x*x;
```

closes vgvassilev#573
vgvassilev pushed a commit that referenced this issue Jun 24, 2023
Earlier for an expression like `(E1, E2)`, the derivative code produced `(dE1, dE2)`,
thus a statement like `double temp = (++x, x*x)` produced:
```cpp
double _d_temp = (_d_x, _d_x * x + x * _d_x);
double temp = (++x, x*x);    <--- original cloned statement
```
This meant that the execution order is `dE1 -> dE2 -> E1 -> E2` (where -> means followed by).

But this doesn't seem right, as `dE2` can depend on `E1`; hence `E1` must be computed before `dE2`.
So, the execution order should be `dE1 -> E1 -> dE2 -> E2`.

This PR generates the derivative code as `(dE1, E1, dE2)`; the original statement is changed to just `E2`.
Note that the result of both expressions is still the same as earlier (i.e. the derivative code will still have the result of `dE2`)

So, the example statement of computing `(x+1)^2`, will produce:
```cpp
double _d_temp = (_d_x, ++x , (_d_x * x + x * _d_x));
double temp = x*x;
```

closes #573
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 a pull request may close this issue.

1 participant