root-project / root

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

PyROOT/cppyy fails with template functions overload with SFINAE #15085

Open VanyaBelyaev opened 8 months ago

VanyaBelyaev commented 8 months ago

Check duplicate issues.

Description

Recent version of PyROOT/cppyy fails for relatively simple templated function overload with SFINAE. The reproducer is here. There is templated C++ class A and there are template functions fun_2 overloaded with SFINAE - see below.

For completeness:

I am using LCG-dev3 cvmfs nightly builds with ROOT 6.31/01

Reproducer

import ROOT
print ( 'ROOT version' , ROOT.gROOT.GetVersion() )
ROOT.gInterpreter.Declare('''
#include <type_traits> 
#include <string> 
template <unsigned int  N>
class A
{
public:
//
template <unsigned int K,
typename std::enable_if<(K<N),int>::type = 0 >
std::string fun_1 () { return "ququ" ;} 
//
template <unsigned int K,
typename std::enable_if<(N<=K),int>::type = 0 >
int         fun_1 () { return K ;} 
};
// the same with functions
template <unsigned int K,unsigned int N, 
typename std::enable_if<(K<N),int>::type = 0 >
inline std::string fun_2 ( A<N>& ) { return "ququ" ;} 
//
template <unsigned int K,unsigned int N,
typename std::enable_if<(N<=K),int>::type = 0 >
inline int         fun_2 ( A<N>& ) { return K ;} 
''')

N = 4 
a = ROOT.A [ N ]()

print ( 'Use templated method of templated class A<4> : it is OK ' ) 
for k in range ( 2 * N + 1 ) : 
    print  ( k , a.fun_1[k]() ) 

print ( 'Use templated function for templated class A<4> : it fails for k >=4' ) 
for k in range( 2 * N + 1  ) :
    func = ROOT.fun_2[k,N]
    print  ( k , func ( a )  ) 

And the output is:

ROOT version 6.31/01
Use templated method of templated class A<4> : it is OK
0 ququ
1 ququ
2 ququ
3 ququ
4 4
5 5
6 6
7 7
8 8
Use templated function for templated class A<4> : it fails for k >=4
0 ququ
1 ququ
2 ququ
3 ququ
Traceback (most recent call last):
  File "/afs/cern.ch/user/i/ibelyaev/cmtuser/RELEASE/ostap/build/./tst64.py", line 44, in <module>
    print  ( k , func ( a )  )
TypeError: Could not find "fun_2<4,4>" (set cppyy.set_debug() for C++ errors):
  Failed to instantiate "fun_2<4,4>(A<4>&)"

ROOT version

6.31/01 Linux, LCG-dev3 cvmvs nightly builds

Installation method

LCG-dev3 cvmfs nightly build

Operating system

Linux

Additional context

If I am not mistaken semtime ago the situations was just an opposite - stanalone template fuctions were OK, but template methods were not OK... but now I am not 100% sure...

guitargeek commented 8 months ago

Hi @VanyaBelyaev!

Can you use C++17? That would simplify the code with if constexpr, and also work in the ROOT nightlies thanks to our cppyy upgrade last week:

// the same with functions
template <unsigned int K,unsigned int N>
auto fun_2 (A<N>&) {
    if constexpr(K < N) return "ququ";
    else return K;

If this is not a solution for you and you absolutely need to support the reproducer above, please also open this issue also in cppyy upstream since its also present there.

If I am not mistaken sometime ago the situations was just an opposite - standalone template functions were OK, but template methods were not OK... but now I am not 100% sure...

What do you mean by "sometime ago"? I checked with ROOT 6.30, and the situation is the same as with ROOT master, meaning the cppyy upgrade didn't cause any regression in this regard.

Anyway, I can't encourage you enough to move to C++17, implementing the patterns that you implement there is a nightmare without if constexpr :slightly_smiling_face:

VanyaBelyaev commented 8 months ago
Hi Jonas, 

Thank you. I'll start to move into the direction with C++17. - but it is a slow process. And for some cases two overloads a bit better (each has aronug 30 lines of unique code.. Better to keep them separated.

"Sometime ago" belongs to arounbg ROOT 6.22 (a really long time ago) - I see there is some swicth in my legacy code..

VanyaBelyaev commented 8 months ago

dear @guitargeek

It seems that my project has one more problem with new cppyy. While I am trying to prepare "easy" reproducer, I need to ask you some advice/recipe. The issue is with "python"-RooAbsPdf. I need to have a RooAbdPdf class with the major method implemented in python. Previously I have such solution, but with new cppyy I've got two problems - first, and the drawing phase, there are error messages that servers are not redirected. ButI have "correct" fit results and the plot.
And, the main problem is that at the end of the script the program stalls - likely in ROOT finalization.
It is not easy to make short, simple & easy reproducer, but I'll try to do it asap. But, might be it is a known issue? What is the "correct/recommended" way for implementation of such "hybrid" pythonic RooAbdPdf?

VanyaBelyaev commented 8 months ago

Hi @guitargeek

I've prepared an relatively easy reproducer with pythonic PDF see this gist

It hangs at the end (the fit it successfull) for ROOT 6.31/01 (LCG dev3 cvmfs nightly slot) but it workd fo the reviosu verisons of ROOT/cppyy.

Thank you very much in advance for advice.

guitargeek commented 8 months ago

Hi @VanyaBelyaev, with the current ROOT nightlies, at least the problem with PyROOT hanging at the end should be gone. Can you confirm?

If it's gone also for you, does it mean your only issue with the new PyROOT at this point is the one you report in the original post of this thread, plus the enum issue?

VanyaBelyaev commented 8 months ago

Dear @guitargeek Thank you very much. In my tests I see positive changes for LCG dev3 cvmfs nightluies

VanyaBelyaev commented 8 months ago

Dear @guitargeek

... and I have one more problem with another test no my project. It also hangs.. Unforunately it i not so simple to isolate,(that's why I am not reporting this problem since I have no simple reproducer) but since it also involves C++ virtual functions reimplemented in python, migth be the unnderlying reason is the same..