Skip to content

Add converters and low-level views for fixed width types#52

Open
guitargeek wants to merge 1 commit into
wlav:masterfrom
guitargeek:fixed-width-converters
Open

Add converters and low-level views for fixed width types#52
guitargeek wants to merge 1 commit into
wlav:masterfrom
guitargeek:fixed-width-converters

Conversation

@guitargeek
Copy link
Copy Markdown
Contributor

@wlav
Copy link
Copy Markdown
Owner

wlav commented Jun 14, 2025

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

@guitargeek
Copy link
Copy Markdown
Contributor Author

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:
root-project/root@7b1108b

@wlav
Copy link
Copy Markdown
Owner

wlav commented Jun 14, 2025

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.

@guitargeek
Copy link
Copy Markdown
Contributor Author

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

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 this pull request may close these issues.

3 participants