root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.7k stars 1.28k forks source link

Non-trivial type traits do not work on cppyy #15062

Open taehyounpark opened 7 months ago

taehyounpark commented 7 months ago

Check duplicate issues.

Description

Dear experts

I wanted to (1) point out two related bugs in ROOT's cppyy, and (2) ask for advice on how such cases are handled by ROOT developers.

First is a minimally reproducible bug involving pretty straightforward type trait gymnastics. The cppyy snippet below seems to simply gives up at the first non-trivial part, and no further Python bindings are possible.

The second, which I suspect is related/stems from the above, is cppyy sometimes "strips" the arguments of a function template parameter. As both pure-C++ and cppyy.cppdef do not make such mistakes (i.e. it's valid C++), I'm not sure what could be going wrong on the bindings-side to result in this behaviour. I have not been able to factorize this part out of my library to make a minimal example, but can be reproduced by with this header.

Neither issue is present in the standalone, newer cppyy 3.0.0, so I suspect updating ROOT's cppyy 1.6.2 would solve the issue. So my call for help is:

Thank you very much for your time!

Reproducer

import cppyy

cppyy.cppdef('''
// some "expression" out of which a std::function can be constructed
template <typename Fn> struct expression {
  using function_type = decltype(std::function(std::declval<Fn>()));
};

// some "equation" specialized to handle such expressions
template <typename T> class equation;
template <typename Ret, typename... Args> class equation<Ret(Args...)> {};

// deduce what equation to make from std::function
template <typename Fn> struct deduce_equation;
template <typename Ret, typename... Args>
struct deduce_equation<std::function<Ret(Args...)>> {
  using type = equation<std::decay_t<Ret>(
  std::decay_t<Args>...)>;
};

// shortcut for expression -> function ->  equation
template <typename Fn>
using equation_t = typename deduce_equation<
    typename expression<Fn>::function_type>::type;

// testing...
auto lmbd = [](int x){return x;};
using simple_equation_t = equation_t<decltype(lmbd)>;
auto simple_equation = simple_equation_t();
''')

# ...it worked
print(cppyy.gbl.simple_equation) # <cppyy.gbl.equation<int(int)> object at 0x108b4d000>

# things stop mid-way in cppyy 
expression_function_type = cppyy.gbl.expression['std::function<int(int)>'].function_type # <class cppyy.gbl.std.function<int(int)> at 0x7fc1ddfa8f30>
simple_deduction = cppyy.gbl.deduce_equation['std::function<int(int)>'].type # AttributeError
# simple_equation = cppyy.gbl.equation_t['std::function<int(int)>']() # works fine with cppyy 3.0.0 from here
# print(simple_equation)
# importing cppyy and my C++ library
import cppyy
cppyy.include('queryosity.h')
from cppyy.gbl import queryosity as q

# JIT and get some C++ function
cppyy.cppdef('''auto fn = std::function([](){return 1;});''')
fn = cppyy.gbl.fn

# use it in my templated library
df = q.dataflow()
expr = q.column.expression['std::function<int()>'](fn) # AttributeError
# one = df._equate(expr) # works fine with cppyy 3.0.0 from here
# print(one) # <cppyy.gbl.queryosity.todo<queryosity::column::evaluator<queryosity::column::equation<int()>>> object at 0x7fc41743b690>

# okay, "persuade" cppyy from C++ side
cppyy.cppdef('''auto expr = queryosity::column::expression(fn);''');
one = df._equate(cppyy.gbl.expr) # worked! ... or did it?
print(one) # <cppyy.gbl.queryosity.todo<queryosity::column::evaluator<queryosity::column::equation<int> > > object at 0x7ff5fefc27d0>
                                                                      #################################
                                                                      # how did int() become int?
                                                                      # more generaly, Ret(Args...) becomes Ret!

ROOT version

   ------------------------------------------------------------------
  | Welcome to ROOT 6.30/04                        https://root.cern |
  | (c) 1995-2024, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for macosx64 on Feb 04 2024, 02:42:45                      |
  | From heads/latest-stable@                                        |
  | With Apple clang version 15.0.0 (clang-1500.1.0.2.5)             |
  | Try '.help'/'.?', '.demo', '.license', '.credits', '.quit'/'.q'  |
   ------------------------------------------------------------------

Installation method

build from source

Operating system

MacOS

Additional context

No response

guitargeek commented 7 months ago

Are there plans/work in progress for ROOT to move to a newer cppyy anytime soon?

Hi @taehyounpark! We have recently upgraded the cppyy frontend in ROOT, which will be part of ROOT 6.32: https://github.com/root-project/root/pull/14507

However, it doesn't fix this reproducer. I suspect that this also requires to synchronize the cppyy-backend. The problem is that this backend is a fork of ROOT itself, including cling. And then, cppyy made patches to this fork of cling/ROOT for e.g. better lambda support and other advanced C++ features and details of the type system.

The problem is that we can't take these patches 1 to 1 back to ROOT, because ROOT also used Cling for other things like IO, and the patches in cppyy did not have to consider compatibility with that.

We try to make cppyy independent of cling on the long term to solve this conundrum. In the meantime, I can see what we are exactly missing in upstream ROOT or Cling to make the reproducer in this issue work. Maybe it is an uncontroversial patch. This will also take some time though. I worked a lot on PyROOT in the last weeks and have to work on other responsibilities in the next weeks before coming back to this. Or maybe @wlav has a hint?

In the meantime, might there be anything I could try to "persuade" similar quirks, from either C++ or Python to make them work?

Unfortunately not. In ROOT, we don't use complicated template code with type traits in user interfaces. That's maybe my recommendation to you: can you simplify the user-facing interface and hide the templated stuff maybe behind some type-erased types or simplify it a bit? IMHO, templates are great for efficient implementations, but for user interfaces it can be a nightmare (think only about the error messages...).

wlav commented 7 months ago

You can look for std::function in the git log, see whether there's anything easy to patch from there.

ROOT likes to strip std::, which is problematic for non-HEP codes, so yes, I've pretty much completely removed that behavior from my fork of ROOT/meta. However, if memory serves, there was something worse with std::function and templates. I think it's this one: https://github.com/wlav/cppyy-backend/commit/98c21db9fad90902efe71d92adc7735cba471c1f

There's also this bug report, which may be related: https://github.com/root-project/root/issues/10680 . (That issue does not exist with cppyy master.)

taehyounpark commented 7 months ago

Thank you @guitargeek and @wlav for the quick and informative responses. These are totally understandable difficulties, and I'm looking forward to the future!

Unfortunately the usage of templates is quite baked-in for this library, which tries to allow users to write their own C++ classes and input (but also retrieve) them in a data analysis workflow. Ultimately, however, I guess this is not a complete showstopper as I could just resort to cppyy.cppdef() where things work, and write a manual wrapper Python interface around it...