Skip to content

PyROOT multidimenstional int16_t arrays stopped working in 6.34 #17841

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
1 task done
cozzyd opened this issue Feb 27, 2025 · 4 comments · Fixed by #18492
Closed
1 task done

PyROOT multidimenstional int16_t arrays stopped working in 6.34 #17841

cozzyd opened this issue Feb 27, 2025 · 4 comments · Fixed by #18492
Assignees
@guitargeek @aaronj0
Labels
bug in:Python Interface
Milestone
6.36.00

Comments

@cozzyd
Copy link
Contributor

cozzyd commented Feb 27, 2025

Check duplicate issues.

  • Checked for duplicates

Description

We serialize some objects containing 2D int16_t arrays (these are waveforms). For our PyROOT interface, we convert them to numpy arrays using numpy.frombuffer.

Starting in ROOT 6.34, at least as distributed by Fedora and by EL9 EPEL, this no longer works. One-dimensional arrays seem fine.

Reproducer

Here is a standalone example exhibiting the problem

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)

This gives something like

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
[[  7424  -5267]
 [-14155 -14397]
 [-11728 -28243]
 [ 32764      0]
 [  2288 -32302]
 [ 32521      0]
 [ 25856 -32290]
 [ 32521      0]
 [  1856 -32275]
 [ 32521      0]
 [     0      0]
 [     0      0]
 [  7424  -5267]
 [-14155 -14397]
 [-11712 -28243]
 [ 32764      0]
 [-29904  25230]
 [ 32521      0]
 [     1      0]
 [     0      0]
 [-13552  -8813]
 [ 32520      0]
 [     0      0]
 [     0      0]]

Obviously the shape and typecode are wrong, and the array is garbage, when this used to work fine.

ROOT version

ROOT 6.34.04

Installation method

dnf

Operating system

Fedora and also EPEL9

Additional context

I reported this at cppyy, but apparently this is fine in current upstream cppyy.
@cozzyd cozzyd added the bug label Feb 27, 2025
@ferdymercury
Copy link
Collaborator

ferdymercury commented Apr 11, 2025 •
edited
Loading

In debug mode I get:

python3: /opt/root_src/interpreter/llvm-project/clang/lib/CodeGen/CGDecl.cpp:160: void clang::CodeGen::CodeGenFunction::EmitDecl(const clang::Decl&): Assertion `VD.isLocalVarDecl() && "Should not see file-scope variables inside a function!"' failed.

@Rajveer100
Copy link

Rajveer100 commented Apr 18, 2025 •
edited
Loading

@ferdymercury
I think we can ignore the assertion in debug mode since this seems to be a clang specific issue with the CodeGenFunction::EmitForStmt since just a simple for loop also throws an error:

VarDecl 0x130ed7908 <input_line_22:10:6, col:14> col:10 used i 'int' cinit
`-IntegerLiteral 0x130ed7970 <col:14> 'int' 0
Assertion failed: (VD.isLocalVarDecl() && "Should not see file-scope variables inside a function!"), function EmitDecl, file CGDecl.cpp, line 162.

...

Looking further in the stack trace, and dumping the Stmt:

VarDecl 0x130ed7908 <input_line_22:10:6, col:14> col:10 used i 'int' cinit
`-IntegerLiteral 0x130ed7970 <col:14> 'int' 0

which is the INIT.

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 < 0; 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)

@Rajveer100
Copy link

This works absolutely fine for int8_t, so it seems like the types are incorrectly matched for some of them in the pyroot types bindings when converting types:

(24, 2)
b
b
[[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]]

@aaronj0 aaronj0 mentioned this issue Apr 24, 2025
Merged
@aaronj0
Copy link
Contributor

aaronj0 commented Apr 24, 2025

A simpler reproducer:

import cppyy
import numpy

cppyy.cppdef("""
 struct Foo {int32_t bar[24][2] = {};};
 Foo foo;
 foo.bar[0][0]=1;
 foo.bar[9][0]=42;
 foo.bar[10][1]=1;
 """)

arr = cppyy.gbl.foo.bar
print(arr)
print(arr.shape)
print(arr.typecode)
print(arr.format)

a = numpy.frombuffer(arr, dtype='int32', count=24*2).reshape((24,2))
print(a)

The following PR: #18492 should fix the problem

@aaronj0 aaronj0 self-assigned this Apr 24, 2025
@aaronj0 aaronj0 added this to the 6.36.00 milestone May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@guitargeek guitargeek

@aaronj0 aaronj0

Labels
bug in:Python Interface
Projects
Fixed in 6.38.00
Status: No status
Fixed in 6.36.00
Status: Done
Milestone
6.36.00
Development

Successfully merging a pull request may close this issue.

[cppyy] Add converters and low-level views for fixed width integers aaronj0/root
5 participants
@guitargeek @cozzyd @ferdymercury @Rajveer100 @aaronj0