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

Incorrect Jacobian generated when Output array is used as a part of arithmetic expressions #480

Closed
Nirhar opened this issue Aug 3, 2022 · 0 comments · Fixed by #1121
Closed

Incorrect Jacobian generated when Output array is used as a part of arithmetic expressions #480

Nirhar opened this issue Aug 3, 2022 · 0 comments · Fixed by #1121
Assignees
@PetroZarytskyi
Milestone
v1.8

Comments

@Nirhar
Copy link
Contributor

Nirhar commented Aug 3, 2022 •
edited
Loading

Minimum reproducible example:

void h(float a, float output[]){
  output[0]=a*a;  
  output[1]=output[0]*3;
}
int main(){
  auto dh = clad::jacobian(h);
  float a=3;
  float output[2]={0};
  float jc[2]={0};
  dh.execute(a,output,jc);
  printf("Result = {%.2f, %.2f}",jc[0],jc[1]);
}

Output: Result = {6.00, 0.00}, which is wrong

The correct Output must be Result = {6.00, 18.00}
@vgvassilev vgvassilev added this to the v1.6 milestone Feb 7, 2024
@vgvassilev vgvassilev modified the milestones: v1.6, v1.7 Jul 11, 2024
@vgvassilev vgvassilev assigned PetroZarytskyi and unassigned vaithak Jul 11, 2024
@vgvassilev vgvassilev modified the milestones: v1.7, v1.8 Aug 4, 2024
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 13, 2024
@PetroZarytskyi
Add a test for vgvassilev#480
426455a
@PetroZarytskyi PetroZarytskyi mentioned this issue Nov 13, 2024
Merged
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 14, 2024
@PetroZarytskyi
Add a test for vgvassilev#480
96eb05d
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 14, 2024
@PetroZarytskyi
Add a test for vgvassilev#480
764adb4
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 17, 2024
@PetroZarytskyi
Add a test for vgvassilev#480
359fff1
vgvassilev pushed a commit to PetroZarytskyi/clad that referenced this issue Nov 19, 2024
@PetroZarytskyi @vgvassilev
Add a test for vgvassilev#480
6812bf7
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 19, 2024
@PetroZarytskyi
Add a test for vgvassilev#480
d990cdb
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 19, 2024
@PetroZarytskyi
Reimplement jacobians using the vectorized forward mode.
dfc7d63 
 Previously, jacobians were based on the non-vectorized reverse mode, which was mostly incapable of capturing multiple outputs. The implementation worked in a few particular cases. For example, it was not possible to differentiate function calls or declare variables inside the original function body.
 This PR implements jacobians using the vectorized forward mode. At the very least, this will solve the issues described above and give a way forward to solve other ones. This also means introducing features to the vectorized fwd mode will introduce the same features to jacobians.
 Let's take a look at the new signature of jacobians. Since the function to be differentiated is expected to have multiple outputs, we should expect the output in the form of array/pointer/reference parameters (just like before). And for every output parameter, we should generate a corresponding adjoint parameter for the user to acquire the results. Since there is no way to specify which parameters are used as output and which are not, adjoints are generated for all array/pointer/reference parameters. For example:

```
void f(double a, double b, double* c)
 -->
void f_jac(double a, double b, double* c, <matrix<double>* _d_c)
```

or

```
void f(double a, double b, double* c, double[7] t)
 -->
void f_jac(double a, double b, double* c, double[7] t,
 array_ref<matrix<double>> _d_c, matrix<double>* _d_t)
```

This signature is also similar to the old one. e.g.
```
df.execute(a, b, c, result); // old behavior
df.execute(a, b, c, &result); // new behavior
```
However, the behavior differs for multiple output parameters, which the old jacobians did not support.

 Note: the same functionality can be achieved by using the vectorized reverse mode, which should probably be implemented at some point. However, the old code for jacobians is unlikely to be useful for that, and there is not much point in keeping it.

Fixes vgvassilev#472, vgvassilev#1057, vgvassilev#480, vgvassilev#527
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 19, 2024
@PetroZarytskyi
Add a test for vgvassilev#480
95cb15a
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 19, 2024
@PetroZarytskyi
Reimplement jacobians using the vectorized forward mode.
6a18cb9 
 Previously, jacobians were based on the non-vectorized reverse mode, which was mostly incapable of capturing multiple outputs. The implementation worked in a few particular cases. For example, it was not possible to differentiate function calls or declare variables inside the original function body.
 This PR implements jacobians using the vectorized forward mode. At the very least, this will solve the issues described above and give a way forward to solve other ones. This also means introducing features to the vectorized fwd mode will introduce the same features to jacobians.
 Let's take a look at the new signature of jacobians. Since the function to be differentiated is expected to have multiple outputs, we should expect the output in the form of array/pointer/reference parameters (just like before). And for every output parameter, we should generate a corresponding adjoint parameter for the user to acquire the results. Since there is no way to specify which parameters are used as output and which are not, adjoints are generated for all array/pointer/reference parameters. For example:

```
void f(double a, double b, double* c)
 -->
void f_jac(double a, double b, double* c, <matrix<double>* _d_c)
```

or

```
void f(double a, double b, double* c, double[7] t)
 -->
void f_jac(double a, double b, double* c, double[7] t,
 array_ref<matrix<double>> _d_c, matrix<double>* _d_t)
```

This signature is also similar to the old one. e.g.
```
df.execute(a, b, c, result); // old behavior
df.execute(a, b, c, &result); // new behavior
```
However, the behavior differs for multiple output parameters, which the old jacobians did not support.

 Note: the same functionality can be achieved by using the vectorized reverse mode, which should probably be implemented at some point. However, the old code for jacobians is unlikely to be useful for that, and there is not much point in keeping it.

Fixes vgvassilev#472, Fixes vgvassilev#1057, Fixes vgvassilev#480, Fixes vgvassilev#527
PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Nov 19, 2024
@PetroZarytskyi
Add a test for vgvassilev#480
0fe5627
vgvassilev pushed a commit that referenced this issue Nov 19, 2024
@PetroZarytskyi @vgvassilev
Add a test for #480
04f2cbb
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.8
Development

Successfully merging a pull request may close this issue.

Reimplement jacobians using the vectorized forward mode PetroZarytskyi/clad
4 participants
@vgvassilev @vaithak @Nirhar @PetroZarytskyi