-
Notifications
You must be signed in to change notification settings - Fork 51
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.
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
CI: Add ADIOS1 (BP3) Python Test #1197
Conversation
Ok, so in
|
I've now fixed the number of chunks issue uncovered by this (simple fix in the tests). Also, this uncovered a long-standing use-after-free bug in ADIOS1, should now also be fixed. |
@@ -531,42 +531,54 @@ load_chunk(RecordComponent & r, py::buffer & buffer, Offset const & offset, Exte | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really know if this part is necessary. But it looks like we forgot it earlier? Copied this from the other overload of load_chunk
.
This fix is irrelevant to the use-after-free issue that I saw.
You seem to be seeing other errors than I did, will address those next. |
These don't look like ADIOS1-specific issues to me. More like somehow pybind appends zeroes to scalars for some reason?? |
4dd9fda
to
22c2b27
Compare
Thanks for the pushes! I will pull and rebased agains the clang-format changes now. |
22c2b27
to
d497560
Compare
pybind/numpy scalars are a bit special: there are zero-dimensional and I think I had to hack a little for them to work. Not sure if the ADIOS1 part contains maybe a work-around because attributes can be arrays or scalars and were patched in relatively late for us... One definitely needs numpy 1.15+ for numpy bugs and there is this helper that would also work well: pybind/pybind11#3544 |
0061bae
to
e5a4718
Compare
Yep, I saw this in the C++ side of the failing functions
It looks to me like the errors happen before ADIOS1 even has the chance to do anything?
|
First error:
For tracing this, I added this to the CI: .def(
"make_constant",
[](RecordComponent &rc, py::buffer &a) {
py::buffer_info buf = a.request();
auto const dtype = dtype_from_bufferformat(buf.format);
using DT = Datatype;
// allow one-element n-dimensional buffers as well
py::ssize_t numElements = 1;
if (buf.ndim > 0)
{
std::cout << "Buffer has dimensionality: " << buf.ndim
<< std::endl;
for (auto d = 0; d < buf.ndim; ++d)
{
std::cout << "Extent of dimensionality " << d << ": "
<< buf.shape.at(d) << std::endl;
numElements *= buf.shape.at(d);
}
} Output is:
|
Second error:
Similar behavior:
|
Third error:
Error is:
|
Summing up the above three messages: pybind11 seems to represent scalar ints as vector of |
Make sure we test ADIOS1 in Python tests.
Don't know if it is necessary, but looks like we forgot it earlier
ADIOS1 removed in 0.15.0+ |
Make sure we test ADIOS1 in Python tests.
We have known issues here that we need to fix.