CI: Add ADIOS1 (BP3) Python Test#1197
Closed
ax3l wants to merge 5 commits intoopenPMD:devfrom
Closed
Conversation
Member
Author
|
Ok, so in |
Contributor
|
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 | |||
| } | |||
| } | |||
|
|
|||
Contributor
There was a problem hiding this comment.
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.
Contributor
|
You seem to be seeing other errors than I did, will address those next. |
Contributor
These don't look like ADIOS1-specific issues to me. More like somehow pybind appends zeroes to scalars for some reason?? |
franzpoeschel
force-pushed
the
ci-addBP3pyTest
branch
from
4dd9fda to
22c2b27
Compare
Member
Author
|
Thanks for the pushes! I will pull and rebased agains the clang-format changes now. |
ax3l
force-pushed
the
ci-addBP3pyTest
branch
from
22c2b27 to
d497560
Compare
Member
Author
|
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 |
ax3l
force-pushed
the
ci-addBP3pyTest
branch
from
0061bae to
e5a4718
Compare
Contributor
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? |
Contributor
|
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: |
Contributor
|
Second error: Similar behavior: |
Contributor
|
Third error: Error is: |
Contributor
|
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
This was referenced Mar 9, 2022
Member
Author
|
ADIOS1 removed in 0.15.0+ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make sure we test ADIOS1 in Python tests.
We have known issues here that we need to fix.