CPyCppyy icon indicating copy to clipboard operation
CPyCppyy copied to clipboard

Add converters and low-level views for fixed width types

Open guitargeek opened this issue 6 months ago • 4 comments

Upstream version of:

  • https://github.com/root-project/root/pull/18492

guitargeek avatar Jun 14 '25 00:06 guitargeek

I'm surprised if this compiles ... I'd be even more surprised if any of these converters are ever called.

wlav avatar Jun 14 '25 04:06 wlav

Well, it compiles and also fixed ROOT issue #17841, so they must be called somewhere no?

Here I copy paste the reproducer from that issue for convenience:

import cppyy
import cppyy.ll
import numpy

cppyy.cppdef("""
 struct Foo {int16_t bar[24][2] = {};};
 Foo foo;
 foo.bar[0][0]=1;
 foo.bar[10][1]=1;
 for(int i = 0; i < 24; i++) { std::cout << foo.bar[i][0] << " " << foo.bar[i][1] << std::endl; }
 """)

arr = cppyy.gbl.foo.bar
print(arr.shape)
print(arr.typecode)
print(arr.format)
# this cast was necessary for some reason in older ROOT versions, though it doesn't change anything here
a = numpy.frombuffer(cppyy.ll.cast['int16_t*'](arr), dtype='int16', count=24*2).reshape((24,2))
print(a)

The output without this patch:

1 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 1
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
(268435455, -1)
L
L
[[-15808  21208]
 [ 32763      0]
 [-27328  18449]
 [ 32767      0]
 [ -1463  21236]
 [ 32763      0]
 [ 16592  21334]
 [ 32763      0]
 [  9344   9286]
 [     0      0]
 [-18624  21215]
 [ 32763      0]
 [ 20656  29840]
 [ 32762      0]
 [ 20656  29840]
 [ 32762      0]
 [ 29312  21138]
 [ 32763      0]
 [ 20656  29840]
 [ 32762      0]
 [ 16592  21334]
 [ 32763      0]
 [ 20656  29840]
 [ 32762      0]]

And with the patch:

1 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 1
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
0 0
(24, 2)
h
h
[[1 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 1]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]
 [0 0]]

Aaron has also written an extensive test for the array converters, that covers also the example from this reproducer: https://github.com/root-project/root/pull/18492/commits/7b1108b609b8f4efdf7dab18efe0c09481fb80ea

guitargeek avatar Jun 14 '25 15:06 guitargeek

Not sure; certainly there are differences: that cppdef crashes in Cling when using master (should be cppexec, although it should crash, of course) and that cast fails to convert but isn't necessary. This runs fine:

import cppyy
import cppyy.ll
import numpy

cppyy.cppexec("""
 struct Foo {int16_t bar[24][2] = {};};
 Foo foo;
 foo.bar[0][0]=1;
 foo.bar[10][1]=1;
 for(int i = 0; i < 24; i++) { std::cout << foo.bar[i][0] << " " << foo.bar[i][1] << std::endl; }
 """)

arr = cppyy.gbl.foo.bar
print(arr.shape)
print(arr.typecode)
print(arr.format)
# this cast was necessary for some reason in older ROOT versions, though it doesn't change anything here
a = numpy.frombuffer(arr, dtype='int16', count=24*2).reshape((24,2))
print(a)

Regardless, I see now what was done: the CreateLowLevelView versions have _i16 etc. appendages to make it compile. Even then, the big difference between int8_t and int16_t is that the former (where possible) is explicitly prevented from resolving (b/c it resolves to a char type, not an int type). No such for the latter (nor any problem with it). Iow., that converter is never called b/c the typedef is resolved.

It is possible that in ROOT has differences in typedef resolution to retain I/O types.

wlav avatar Jun 14 '25 22:06 wlav

Thanks for the insights! Right, ROOT resolves the typedefs differently. So hopefully, we don't need this patch anymore once we have libInterOp :slightly_smiling_face:

guitargeek avatar Jun 14 '25 23:06 guitargeek