clad icon indicating copy to clipboard operation
clad copied to clipboard

Functions with pointer assigns lead to compilation error in reverse mode

Open grimmmyshini opened this issue 3 years ago • 6 comments

Any assignment of pointers results in a compilation error. Minimum reproducable example:

double f5(double* p) {
  double* another_ptr = p;
  return *another_ptr;
}
clad::gradient(f5);

results in

error: invalid operands to binary expression ('double *' and 'double *')

Ideally, these expressions should be ignored and warned against, they should still be compiled however. I suspect some issue with the _result type mismatch with the intermediate pointers, however, I am not sure. This issue is also closely related to #195 and solving that might also solve this one.

This and #195 both work towards adding array/reference differentiability support too and maybe solved as that feature is addressed.

Another example to reproduce the same error is:

double f5(double a) {
  double* ptr, *another_ptr;
  ptr =  another_ptr;
  return *ptr;
}

results in the same error when executed in reverse mode. It works as expected in the forward mode.

grimmmyshini avatar Feb 23 '21 13:02 grimmmyshini

Ideally, how should functions containing pointer parameters should be differentiated ? And differentiating this function in forward mode is giving error too

double f5(double* p) {
  double* another_ptr = p;
  return *another_ptr;
}
int main() {
    clad::differentiate(f5);
}
error: attempted differentiation w.r.t. a parameter ('p') which is not of a real type
double f5(double* p) {

Error obtained using gradient function is not as friendly as the one obtained using differentiation. But why should the code still be compiled ?

parth-07 avatar Feb 24 '21 19:02 parth-07

Clad does not yet support multi-arg calls (here pointer equivalent) in forward mode. That means while it won't support any non-real argument, it supports pointers being used in the function body as dependent variables/constants. And if we encounter some pointer arithmetic we do not support, we should ideally ignore it and issue a warning stating that generation of a derivative of that operation is not yet supported.

The ideal result of the function causing the issue should just be that it compiles. Maybe there are warnings emitted, but in no case should it result in an error as it does now, especially since the program is syntactically correct. The derivative will not necessarily be correct but that is okay.

As for differentiating function with pointer inputs. The pointers are treated mostly as signifying an array (we don't currently support using pointers with the * notation i.e *(p + 1) = 34 is not supported but p[1] = 34 is supported) and are differentiated accordingly.

grimmmyshini avatar Feb 25 '21 06:02 grimmmyshini

Thank you for your detailed reply, I understand the situation much better now. I am having one more doubt, unrelated to this discussion but I am not sure where to ask this, If I am having some doubt/confusion regarding clad codebase, where can I ask about them ?

parth-07 avatar Feb 25 '21 08:02 parth-07

You can ask here for now, but on that note, @vgvassilev what about a discussions tab on the repo?

grimmmyshini avatar Feb 25 '21 08:02 grimmmyshini

@parth-07, I would the diagnostics on calling clad::differentiate(f5); a feature ;) We should not use clad::differentiate for array types because clad::gradient is far more efficient. That being said, we might want to do two things to make this clearer:

  • Improve the diagnostics & improve documentation;
  • Allow users to specify that they want to use clad::differentiate on double/float* as they are not array types but just pointers to a single value.clad::differentiate<single_value>(f5). I'd be inclined to support that case only if there is a compelling real world use case for it.

You can open an issue if something is unclear about the codebase. @grimmmyshini I will have to learn more about "Discussions" and maybe that's the longer term answer to that question.

vgvassilev avatar Feb 25 '21 09:02 vgvassilev

@vgvassilev @grimmmyshini I am playing with the clad codebase from last couple of days to understand it better, I am also studying about clang plugins and clang AST, hoping understanding them better will help me to understand the codebase better. I will open an issue if something is unclear, thank you for the supportive responses.

parth-07 avatar Feb 25 '21 09:02 parth-07