C++ interface templates appear to be broken

Question

C++ interface templates appear to be broken

GregTheMadMonk opened this issue 4 months ago · 9 comments

Hello! Playing with C++ interface trying to get it to work and this simple program:

#include <vector>

#include <enzyme/enzyme>

using Flt = double;
using Vec = std::vector<Flt>;

Flt sum(const Vec& x) {
    Flt ret = 0.0;
    for (const auto xi : x) ret += xi;
    return ret;
} // <-- sum

Vec dsum(const Vec& x) {
    Vec dx(x.size());
    return enzyme::get<0>(
        enzyme::autodiff<enzyme::Forward>(
            sum,
            enzyme::DuplicatedNoNeed<const Vec&>{ x, dx }
        )
    );
} // <-- dsum

int main() {
    return 0;
}

doesn't compile with the following errors:

In file included from /media/ssd/projects/cpp/enzyme/enzyme-tests/cpp-interface-test/main.cc:3:
In file included from /enzymeroot/enzyme/enzyme:3:
/enzymeroot/enzyme/utils:68:7: error: multiple overloads of 'DuplicatedNoNeed' instantiate to the same signature 'void (const std::vector<double> &, const std::vector<double> &)'
   68 |       DuplicatedNoNeed(T&& v, T&& s) : value(v), shadow(s) {};
      |       ^
/media/ssd/projects/cpp/enzyme/enzyme-tests/cpp-interface-test/main.cc:19:13: note: in instantiation of template class 'enzyme::DuplicatedNoNeed<const std::vector<double> &>' requested here
   19 |             enzyme::DuplicatedNoNeed<const Vec&>{ x, dx }
      |             ^
/enzymeroot/enzyme/utils:67:7: note: previous declaration is here
   67 |       DuplicatedNoNeed(const T& v, const T& s) : value(v), shadow(s) {};
      |       ^
In file included from /media/ssd/projects/cpp/enzyme/enzyme-tests/cpp-interface-test/main.cc:1:
In file included from /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/vector:86:
In file included from /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/bits/memory_resource.h:41:
In file included from /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/bits/uses_allocator_args.h:39:
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/tuple:2672:45: error: implicit instantiation of undefined template 'std::tuple_size<enzyme::tuple<int *&&, int &&, const std::vector<double> &&, const std::vector<double> &&>>'
 2672 |     : __make_tuple_impl<0, tuple<>, _Tuple, tuple_size<_Tuple>::value>
      |                                             ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/tuple:2678:14: note: in instantiation of template class 'std::__do_make_tuple<enzyme::tuple<int *&&, int &&, const std::vector<double> &&, const std::vector<double> &&>>' requested here
 2678 |     : public __do_make_tuple<__remove_cvref_t<_Tuple>>
      |              ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/tuple:2709:19: note: in instantiation of template class 'std::__make_tuple<enzyme::tuple<int *&&, int &&, const std::vector<double> &&, const std::vector<double> &&>>' requested here
 2709 |         <typename __make_tuple<_Tpls>::__type...>::__type __type;
      |                   ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/tuple:2776:17: note: in instantiation of template class 'std::__tuple_cat_result<enzyme::tuple<int *&&, int &&, const std::vector<double> &&, const std::vector<double> &&>>' requested here
 2776 |     -> typename __tuple_cat_result<_Tpls...>::__type
      |                 ^
/enzymeroot/enzyme/tuple:127:10: note: while substituting deduced template arguments into function template 'tuple_cat' [with _Tpls = <tuple<int *&&, int &&, const vector<double, allocator<double>> &&, const vector<double, allocator<double>> &&>>]
  127 |   return tuple_cat(concat_with_fwd_tuple< iseq<FWD_TUPLE>, iseq<first> >::f(impl::forward<FWD_TUPLE>(fwd), impl::forward<first>(t)), impl::forward<rest>(ts)...);
      |          ^
/enzymeroot/enzyme/tuple:135:16: note: in instantiation of function template specialization 'enzyme::impl::tuple_cat<enzyme::tuple<int *>, enzyme::tuple<int, const std::vector<double> &, const std::vector<double> &>>' requested here
  135 |   return impl::tuple_cat(impl::forward<Tuples>(tuples)...);
      |                ^
/enzymeroot/enzyme/utils:426:120: note: in instantiation of function template specialization 'enzyme::tuple_cat<enzyme::tuple<int *>, enzyme::tuple<int, const std::vector<double> &, const std::vector<double> &>>' requested here
  426 |         return autodiff_impl<return_type, DiffMode, function, functy, RetActivity>(impl::forward<function>(f), enzyme::tuple_cat(enzyme::tuple{detail::ret_used<DiffMode, RetActivity>::value}, expand_args(args)...));
      |                                                                                                                        ^
/media/ssd/projects/cpp/enzyme/enzyme-tests/cpp-interface-test/main.cc:17:17: note: in instantiation of function template specialization 'enzyme::autodiff<enzyme::ForwardMode, double (&)(const std::vector<double> &), enzyme::DuplicatedNoNeed<const std::vector<double> &>>' requested here
   17 |         enzyme::autodiff<enzyme::Forward>(
      |                 ^
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/14.1.1/../../../../include/c++/14.1.1/bits/utility.h:49:12: note: template is declared here
   49 |     struct tuple_size;
      |            ^
In file included from /media/ssd/projects/cpp/enzyme/enzyme-tests/cpp-interface-test/main.cc:3:
In file included from /enzymeroot/enzyme/enzyme:3:
/enzymeroot/enzyme/utils:407:82: error: static_cast from 'double (*)(const std::vector<double> &)' to 'double (*)(std::vector<double>)' is not allowed
  407 |       return detail::autodiff_apply<DiffMode>::template impl<return_type>((void*)static_cast<functy*>(f), detail::ret_global<RetActivity>::value, impl::forward<Tuple>(arg_tup), std::make_index_sequence<enzyme::tuple_size_v<Tuple>>{});
      |                                                                                  ^~~~~~~~~~~~~~~~~~~~~~~
/enzymeroot/enzyme/utils:426:16: note: in instantiation of function template specialization 'enzyme::autodiff_impl<enzyme::tuple<double>, enzyme::ForwardMode, double (&)(const std::vector<double> &), double (std::vector<double>), enzyme::DuplicatedNoNeed<double>, int *, int, std::vector<double>, std::vector<double>, 0>' requested here
  426 |         return autodiff_impl<return_type, DiffMode, function, functy, RetActivity>(impl::forward<function>(f), enzyme::tuple_cat(enzyme::tuple{detail::ret_used<DiffMode, RetActivity>::value}, expand_args(args)...));
      |                ^
/media/ssd/projects/cpp/enzyme/enzyme-tests/cpp-interface-test/main.cc:17:17: note: in instantiation of function template specialization 'enzyme::autodiff<enzyme::ForwardMode, double (&)(const std::vector<double> &), enzyme::DuplicatedNoNeed<const std::vector<double> &>>' requested here
   17 |         enzyme::autodiff<enzyme::Forward>(
      |                 ^
3 errors generated.

It seems that the interface

Does not quite like the use of references :)
Uses some std:: stuff with an internal enzyme::tuple

Unless I terribly misunderstand something, this is an error.

If it is, I think I'm going to try and fix it

edit: btw, what version of C++ should I use if editing the headers? C++17, or do you target lower versions too?

Answer 1 · 2024-05-24T04:29:27.000Z

Yeah it is known that references aren't handled properly in the new interface, help definitely welcome!

And yeah tentaitlvely c++17, though obviously earlier support would be nice if possible

Answer 2 · 2024-06-02T15:34:51.000Z

Adding a MFE which used to work in the original PR for the C++ interface https://fwd.gymni.ch/nsKPWF

Answer 3 · 2024-06-02T21:23:02.000Z

@jandrej with #1914 your code compiles with the following minor changes:

62,64c62,63
<    return std::tuple<enzyme::Duplicated<decltype(std::get<Is>(args))>...>{
<       { std::get<Is>(args), std::get<Is>(shadow_args) }...
<    };
---
>    return std::tuple_cat(std::make_tuple(
>                             enzyme::Duplicated<std::remove_cv_t<std::remove_reference_t<decltype(std::get<Is>(args))>>*> {&std::get<Is>(args), &std::get<Is>(shadow_args)})...);
87c86
<                 enzyme::autodiff<enzyme::Forward>
---
>                 enzyme::autodiff<enzyme::Forward, enzyme::DuplicatedNoNeed<kf_return_t>>
106,110c105
<     std::get<0>(kernel_args) = 3;
<     std::get<0>(kernel_shadow_args) = 1;
<
<     const auto res = fwddiff_apply_enzyme(func, kernel_args, kernel_shadow_args);
<     std::cout << res << " == 6\n";
---
>     fwddiff_apply_enzyme(func, kernel_args, kernel_shadow_args);

(I've replaced std::make_tuple(std::tuple_cat(... with a std::tuple constructor to make it compile with C++23, which I use. Note that the enzyme::Duplicated argument has also changed from the pointer to a reference. Waiting for a response on my PR to know if I understood Duplicated/DuplicatedNoNeed right).

It still doesn't work as intended (prints 0 == 6), since, as I've discovered, the C++ interface also doesn't process function pointers correctly. For example, this code

#include <enzyme/enzyme>
#include <iostream>

double f(const double& x) { return x * x; }

int main() {
    double x = 3, dx = 0;
    enzyme::autodiff<enzyme::Reverse>(
        f, enzyme::Duplicated<const double&>{ x, dx }
    );
    std::cout << x << ' ' << dx << '\n';
    return 0;
}

will compile and work meanwhile this:

#include <enzyme/enzyme>
#include <iostream>

double f(const double& x) { return x * x; }

int main() {
    double x = 3, dx = 0;
    enzyme::autodiff<enzyme::Reverse>(
        &f, enzyme::Duplicated<const double&>{ x, dx }
    );
    std::cout << x << ' ' << dx << '\n';
    return 0;
}

will not:

error: Enzyme: No create nofree of unknown value
ptr %2
 at context:   %4 = tail call noundef double %3(ptr noundef nonnull align 8 dereferenceable(8) %0) #7
1 error generated.
make[2]: *** [CMakeFiles/test.dir/build.make:76: CMakeFiles/test.dir/main.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:910: CMakeFiles/test.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

(nevermind the line numbers, I'm testing everything in a single main.cc with a few #if 0 ... #endif's)

Answer 4 · 2024-06-02T23:17:15.000Z

@GregTheMadMonk the issue is that we need to ensure that all the reference passes do not end up creating a copy and thus the autodiff/fwddiff call gets the pointer to the actual reference provided.

Re function pointers, they are supported, but when passed directly. (like the top)

Answer 5 · 2024-06-02T23:36:48.000Z

@wsmoses Sorry, I'm not sure I quite follow what you're saying... Do you mean that we need to ensure that the pointer __enzyme_autodiff/__enzyme_fwddiff end up getting point to the same variables the user has provided when they have called the code? In that case there indeed will be no copying of data, only of references and pointer to it (but I'll re-check it).

Then there is also the issue that I've written about in the PR comments - but I see you've already read and replied to it :)

And I also realize (now) why function pointers can't really be processed (since a function pointer could point to any function really and there is no guarantee it was processed by Enzyme). __enzyme_autodiff can process some though (e.g. when passing a compile-time known function pointer via &f or +lambda). Should this be preserved, or outright prohibited (ideally via a static_assert with a message explaining why function pointers are not to be used in this context and what could be used instead)?

It seems that the best self-critique arrives after the code has been submitted...

It's getting quite late, I won't be replying for a while

Answer 6 · 2024-06-02T23:43:55.000Z

So we can find out what's happening by looking at the generated LLVM. If there's a copy somewhere we'll see a different allocation passed to the underlying __enzyme_autodiff (which is my guess why it could get a different answer than expected).

It would be nice to do a check at the outermost c++ api level (though we have some internal shenanigans to handle a slightly wider scope).

Answer 7 · 2024-06-03T03:27:02.000Z

@GregTheMadMonk did you check the output values? I only get zeros from the fwddiff in your branch.
edit:
Just understood your comment now. I hope you find the problem, I'm happy to test.

Answer 8 · 2024-07-22T21:47:42.000Z

Seemingly resolved.

Answer 9 · 2024-07-23T06:49:52.000Z

Thank you! Sorry for being very inconsistent with GH activity