xsuite / xobjects

Library to create and manipulate serialized objects in CPU and GPU memory and to compile code on these platforms.
https://xsuite.readthedocs.io
Apache License 2.0
4 stars 15 forks source link

Field alignment problem in struct? #137

Open szymonlopaciuk opened 1 month ago

szymonlopaciuk commented 1 month ago

I defined a Quadrupole element with some uint8 fields (see https://github.com/xsuite/xtrack/blob/e7ac347130b09ae1208061ba34b58e5e6d62f19f/xtrack/beam_elements/elements.py#L1194) and xtrack/tests/test_elements.py::test_constructors fails when dumping and reloading the xobject (on OpenCL).

From inspecting the buffers all the meaningful data seems to actually be in the right place (bytes at 144 and 152 are zero). However, it's not clear to me if this is by chance (and we have an alignment problem) or if it's simply the fact that the buffer is not being zeroed by default on OpenCL (then we have less of a problem but we cannot assume anymore that two identical xobjects are bitwise equal).

This is the exact difference and layout:

(Pdb) ee._xobject._buffer.buffer[ee._xobject._offset:ee._xobject._offset + ee._xobject._size].reshape([25, 8])
array([[   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,  -16,   63],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   5,    0,    0,    0,    0,    0,    0,    0],
       [  17,   17,   17,   17,   17,   17, -127,   63],
       [   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,    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,    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,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,  -66,  -66,  -66,  -66,  -66,  -66,  -66],
       [   0,  -66,  -66,  -66,  -66,  -66,  -66,  -66],
       [   0,    0,    0,    0,    0,   56, -113,  -64],
       [   0,    0,    0,    0,    0,   56, -113,  -64],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0]], dtype=int8)
(Pdb) nee._xobject._buffer.buffer[nee._xobject._offset:nee._xobject._offset + nee._xobject._size].reshape([25, 8])
array([[   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,  -16,   63],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   5,    0,    0,    0,    0,    0,    0,    0],
       [  17,   17,   17,   17,   17,   17, -127,   63],
       [   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,    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,    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,    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],
       [   0,   17,   17,   17,   17,   17, -127,   63],
       [   0,    0,    0,    0,    0,   56, -113,  -64],
       [   0,    0,    0,    0,    0,   56, -113,  -64],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0],
       [   0,    0,    0,    0,    0,    0,    0,    0]], dtype=int8)
(Pdb) pp xt.Quadrupole._XoStruct._fields
[<field0 k1 at 0>,
 <field1 k1s at 8>,
 <field2 length at 16>,
 <field3 num_multipole_kicks at 24>,
 <field4 order at 32>,
 <field5 inv_factorial_order at 40>,
 <field6 knl at 48>,
 <field7 ksl at 96>,
 <field8 edge_entry_active at 144>,
 <field9 edge_exit_active at 152>,
 <field10 _sin_rot_s at 160>,
 <field11 _cos_rot_s at 168>,
 <field12 _shift_x at 176>,
 <field13 _shift_y at 184>,
 <field14 _shift_s at 192>]
rdemaria commented 1 month ago

In general we don't zero buffers. If a type leaves some bytes not declared, it should be expected that this portion of the data can contain random bytes. Where do we use bit wise identity?

szymonlopaciuk commented 1 month ago

I don't think we ever do it in production code, the error I saw happens in the test xtrack/tests/test_elements.py::test_constructors. I circumvented the problem by using a uint64, which we do anyway in many other places.

I mean you're right bitwise comparison is not the right way of checking the structs are the same if we allow them to be a non-contiguous type. However, in that test we assert the memory layout since we test json dumping, so we don't have much else to go of off.

Do you think zeroing the padding by default (on OpenCL, since the rest seem to be doing it already) would be a significant overhead?

rdemaria commented 1 month ago

Do you mean initialize the opencl buffer with zeros? I don't it will cost much.