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

Regression in support of functions with unknown definition #890

Closed
guitargeek opened this issue May 13, 2024 · 4 comments · Fixed by #897
Closed

Regression in support of functions with unknown definition #890

guitargeek opened this issue May 13, 2024 · 4 comments · Fixed by #897
Assignees
Milestone

Comments

@guitargeek
Copy link

Clad doesn't work with functions where is doesn't know the definition anymore.

Reproducer:

// clang++ -fPIC mylib.cxx -c -o mylib.o && ar r libmylib.a mylib.o

double foo(double x, double alpha, double theta, double x0 = 0)
{
   return x * alpha * theta * x0;
}
// Compile with clang++ -O2 -std=c++20 -fplugin=/usr/lib/clad.so repro.cxx -o repro -lmylib -L.

#include "clad/Differentiator/Differentiator.h"

#include <iostream>
#include <vector>

double foo(double x, double alpha, double theta, double x0 = 0);

double wrapper(double *params)
{
   const double ix = 1 + params[0];
   return foo(10., ix, 1.0);
}

int main()
{
   std::vector<double> params{4};

   std::cout << "Value" << std::endl;
   const double nllVal = wrapper(params.data());
   std::cout << nllVal << std::endl;
   std::cout << std::endl;

   auto grad = clad::gradient(wrapper, "params");
}

The error:

warning: Falling back to numerical differentiation for 'foo' since no suitable overload was found and clad could not derive it. To disable this feature, compile your programs with -DCLAD_NO_NUM_DIFF.
In file included from repro.cxx:3:
/usr/include/clad/Differentiator/Differentiator.h:62:12: error: no viable conversion from returned value of type 'const double *' to function return type 'clad::array_ref<double>'
   62 |     return val;
      |            ^~~
note: in instantiation of function template specialization 'clad::push<double, const double *>' requested here
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'const double *' to 'const array_ref<double> &' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'const double *' to 'array_ref<double> &&' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:32:20: note: candidate constructor not viable: 1st argument ('const double *') would lose const qualifier
   32 |   CUDA_HOST_DEVICE array_ref(T* a) : m_arr(a), m_size(1) {}
      |                    ^         ~~~~
/usr/include/clad/Differentiator/ArrayRef.h:34:20: note: candidate constructor not viable: no known conversion from 'const double *' to 'array<double> &' for 1st argument
   34 |   CUDA_HOST_DEVICE array_ref(array<T>& a) : m_arr(a.ptr()), m_size(a.size()) {}
      |                    ^         ~~~~~~~~~~~
In file included from repro.cxx:3:
In file included from /usr/include/clad/Differentiator/Differentiator.h:17:
In file included from /usr/include/clad/Differentiator/NumericalDiff.h:7:
/usr/include/clad/Differentiator/Tape.h:56:11: error: no matching constructor for initialization of 'clad::array_ref<double>'
   56 |           T(std::forward<ArgsT>(args)...);
      |           ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/clad/Differentiator/Differentiator.h:61:8: note: in instantiation of function template specialization 'clad::tape_impl<clad::array_ref<double>>::emplace_back<const double *&>' requested here
   61 |     to.emplace_back(val);
      |        ^
note: in instantiation of function template specialization 'clad::push<double, const double *>' requested here
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'const double *' to 'const array_ref<double>' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'const double *' to 'array_ref<double>' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:32:20: note: candidate constructor not viable: 1st argument ('const double *') would lose const qualifier
   32 |   CUDA_HOST_DEVICE array_ref(T* a) : m_arr(a), m_size(1) {}
      |                    ^         ~~~~
/usr/include/clad/Differentiator/ArrayRef.h:34:20: note: candidate constructor not viable: no known conversion from 'const double *' to 'array<double> &' for 1st argument
   34 |   CUDA_HOST_DEVICE array_ref(array<T>& a) : m_arr(a.ptr()), m_size(a.size()) {}
      |                    ^         ~~~~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:25:3: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
   25 |   array_ref() = default;
      |   ^
/usr/include/clad/Differentiator/ArrayRef.h:28:20: note: candidate constructor not viable: requires 2 arguments, but 1 was provided
   28 |   CUDA_HOST_DEVICE array_ref(T* arr, std::size_t size)
      |                    ^         ~~~~~~~~~~~~~~~~~~~~~~~~
In file included from repro.cxx:3:
/usr/include/clad/Differentiator/Differentiator.h:62:12: error: no viable conversion from returned value of type 'int *' to function return type 'clad::array_ref<double>'
   62 |     return val;
      |            ^~~
note: in instantiation of function template specialization 'clad::push<double, int *>' requested here
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int *' to 'const array_ref<double> &' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'int *' to 'array_ref<double> &&' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:32:20: note: candidate constructor not viable: no known conversion from 'int *' to 'double *' for 1st argument
   32 |   CUDA_HOST_DEVICE array_ref(T* a) : m_arr(a), m_size(1) {}
      |                    ^         ~~~~
/usr/include/clad/Differentiator/ArrayRef.h:34:20: note: candidate constructor not viable: no known conversion from 'int *' to 'array<double> &' for 1st argument
   34 |   CUDA_HOST_DEVICE array_ref(array<T>& a) : m_arr(a.ptr()), m_size(a.size()) {}
      |                    ^         ~~~~~~~~~~~
In file included from repro.cxx:3:
In file included from /usr/include/clad/Differentiator/Differentiator.h:17:
In file included from /usr/include/clad/Differentiator/NumericalDiff.h:7:
/usr/include/clad/Differentiator/Tape.h:56:11: error: no matching constructor for initialization of 'clad::array_ref<double>'
   56 |           T(std::forward<ArgsT>(args)...);
      |           ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/clad/Differentiator/Differentiator.h:61:8: note: in instantiation of function template specialization 'clad::tape_impl<clad::array_ref<double>>::emplace_back<int *&>' requested here
   61 |     to.emplace_back(val);
      |        ^
note: in instantiation of function template specialization 'clad::push<double, int *>' requested here
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int *' to 'const array_ref<double>' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:16:29: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'int *' to 'array_ref<double>' for 1st argument
   16 | template <typename T> class array_ref {
      |                             ^~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:32:20: note: candidate constructor not viable: no known conversion from 'int *' to 'double *' for 1st argument
   32 |   CUDA_HOST_DEVICE array_ref(T* a) : m_arr(a), m_size(1) {}
      |                    ^         ~~~~
/usr/include/clad/Differentiator/ArrayRef.h:34:20: note: candidate constructor not viable: no known conversion from 'int *' to 'array<double> &' for 1st argument
   34 |   CUDA_HOST_DEVICE array_ref(array<T>& a) : m_arr(a.ptr()), m_size(a.size()) {}
      |                    ^         ~~~~~~~~~~~
/usr/include/clad/Differentiator/ArrayRef.h:25:3: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
   25 |   array_ref() = default;
      |   ^
/usr/include/clad/Differentiator/ArrayRef.h:28:20: note: candidate constructor not viable: requires 2 arguments, but 1 was provided
   28 |   CUDA_HOST_DEVICE array_ref(T* arr, std::size_t size)
      |                    ^         ~~~~~~~~~~~~~~~~~~~~~~~~
1 warning and 4 errors generated.

The warning about fallback to numeric differentiation is correct. But aftterwards, it should not crash.

@vgvassilev
Copy link
Owner

@vaithak, could that be related to the recent changes we did in forward declarations first approach?

@vaithak
Copy link
Collaborator

vaithak commented May 13, 2024

@vaithak, could that be related to the recent changes we did in forward declarations first approach?

Maybe, I will take a look at this soon.

@vgvassilev vgvassilev added this to the v1.5 milestone May 13, 2024
@vgvassilev
Copy link
Owner

That seems critical for releasing v1.5.

@vaithak
Copy link
Collaborator

vaithak commented May 14, 2024

I tested it now, and this fails exactly after the #802. @PetroZarytskyi, can you take a look once?

@vaithak vaithak linked a pull request May 20, 2024 that will close this issue
vgvassilev pushed a commit that referenced this issue May 20, 2024
- Add custom derivative to DerivativeSet: This is required if the definition of the custom derivative is not found in the current translation unit and is linked in from another.
Adding it to the set of derivatives ensures that the custom derivative is not differentiated again using numerical differentiation due to an unavailable definition.

- Fix recursive processing of DiffRequests: There can be cases where `m_Multiplexer` is not provided. Hence, we don't delay HandleTranslationUnit at the end and it is called repeatedly. This resulted in HandleTopLevelDecl being called recursively (from PerformPendingInstantiations). This commit adds conditional checks to ensure this doesn't perturb the execution of the differentiation plan.

Fixes: #890
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.

3 participants