clad Write numerical diff results to c-style arrays in the generated code.

Currently, the numerical diff generated inside gradients is unable to handle different type arguments. As an example, let's differentiate a call to "double ff(double, unsigned int);":

x = ff(a, n);

-->

...
clad::tape<clad::array_ref<double> > _t1 = {};
double _grad0 = 0;
clad::push(_t1, &_grad0);
unsigned int _grad1 = 0;
clad::push(_t1, &_grad1);
numerical_diff::central_difference(ff, _t1, 0, a, n);
...

So &_grad1(unsigned int*) is pushed to _t1 (clad::tape<clad::array_ref<double>>). The obvious solution here would be to only use the type of the tape element. In this case, there's no point in creating separate _grad, it would make more sense to create an array. This is the solution suggested in this PR:

x = ff(a, n);

-->

 double _grad0[2] = {0};
 numerical_diff::central_difference(ff, _grad0, 0, a, n);

One could argue this will only work for numerical type arguments but other types are not supported in reverse mode numerical diff anyway.

May 07 '24 12:05 PetroZarytskyi

@mcbarton, looks like we got Error: Unsupported version for platform (os=linux, arch=x64, version=18.1.3)! for the clang-tidy check.

May 07 '24 13:05 vgvassilev

@vgvassilev @PetroZarytskyi The latest commit added support for 18.1.3 https://github.com/KyleMayes/install-llvm-action/commits/master/ . It can also be seen here for v2.0.2 release. Try changing @v2 to @v2.0.2 .

May 07 '24 13:05 mcbarton

@vgvassilev I'm not sure why that didn't work. It is supported according to the github action release. Try an earlier version of 18, or another version of llvm to see what happens.

May 07 '24 18:05 mcbarton

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.82%. Comparing base (1011b13) to head (e3465eb).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #883      +/-   ##
==========================================
- Coverage   94.83%   94.82%   -0.01%     
==========================================
  Files          52       52              
  Lines        7719     7708      -11     
==========================================
- Hits         7320     7309      -11     
  Misses        399      399

Files	Coverage Δ
lib/Differentiator/ReverseModeVisitor.cpp	`97.35% <100.00%> (-0.02%)`	:arrow_down:

Files	Coverage Δ
lib/Differentiator/ReverseModeVisitor.cpp	`97.35% <100.00%> (-0.02%)`	:arrow_down:

May 07 '24 18:05 codecov[bot]

@vgvassilev I'm not sure why that didn't work. It is supported according to the github action release. Try an earlier version of 18, or another version of llvm to see what happens.

These things are a can of worms. Maybe we should move to the earliest version possible?

May 07 '24 18:05 vgvassilev

https://github.com/KyleMayes/install-llvm-action/releases/tag/v2.0.0 v2 removed support for llvm 7, so any version after that should work @vgvassilev

May 07 '24 19:05 mcbarton

Maybe trying to bump it to 16 if we were using 15?

May 07 '24 19:05 vgvassilev

@vgvassilev Its probably best for now to revert the changes with regards to clang tidy. I will do some experimention on how to fix this in CppInterOp . I will modify the code to remove the warnings when building, which should activate the clang tidy workflow and allow me to be sure it works before committing it.

May 07 '24 20:05 mcbarton