Skip to content
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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Feb 8, 2022

Make sure we test ADIOS1 in Python tests.

We have known issues here that we need to fix.

@ax3l
Copy link
Member Author

ax3l commented Feb 16, 2022

Ok, so in Unittest.py these tests fail:

======================================================================
ERROR: testConstantRecords (API.APITest.APITest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 570, in testConstantRecords
    self.makeConstantRoundTrip(ext)
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 427, in makeConstantRoundTrip
    ms["int16"][SCALAR].make_constant(np.int16(234))
RuntimeError: make_constant: Only scalar values supported!

======================================================================
ERROR: testParticlePatches (API.APITest.APITest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1643, in testParticlePatches
    self.backend_particle_patches(ext)
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1591, in backend_particle_patches
    e.particle_patches["numParticles"][SCALAR].store(0, np.uint64(10))
RuntimeError: store: Only scalar values supported!

======================================================================
FAIL: testAttributes (API.APITest.APITest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 384, in testAttributes
    self.attributeRoundTrip(ext)
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 277, in attributeRoundTrip
    self.assertEqual(series.get_attribute("int16"), 234)
AssertionError: [234, 0] != 234

----------------------------------------------------------------------

@franzpoeschel
Copy link
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
}
}

Copy link
Contributor

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.

@franzpoeschel
Copy link
Contributor

You seem to be seeing other errors than I did, will address those next.

@franzpoeschel
Copy link
Contributor

Ok, so in Unittest.py these tests fail:

======================================================================
ERROR: testConstantRecords (API.APITest.APITest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 570, in testConstantRecords
    self.makeConstantRoundTrip(ext)
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 427, in makeConstantRoundTrip
    ms["int16"][SCALAR].make_constant(np.int16(234))
RuntimeError: make_constant: Only scalar values supported!

======================================================================
ERROR: testParticlePatches (API.APITest.APITest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1643, in testParticlePatches
    self.backend_particle_patches(ext)
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1591, in backend_particle_patches
    e.particle_patches["numParticles"][SCALAR].store(0, np.uint64(10))
RuntimeError: store: Only scalar values supported!

======================================================================
FAIL: testAttributes (API.APITest.APITest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 384, in testAttributes
    self.attributeRoundTrip(ext)
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 277, in attributeRoundTrip
    self.assertEqual(series.get_attribute("int16"), 234)
AssertionError: [234, 0] != 234

----------------------------------------------------------------------

These don't look like ADIOS1-specific issues to me. More like somehow pybind appends zeroes to scalars for some reason??
I'll push a commit for some printf CI debugging, can't reproduce this locally.

@ax3l
Copy link
Member Author

ax3l commented Feb 18, 2022

Thanks for the pushes! I will pull and rebased agains the clang-format changes now.

@ax3l ax3l mentioned this pull request Feb 18, 2022
7 tasks
@ax3l
Copy link
Member Author

ax3l commented Feb 18, 2022

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

@franzpoeschel
Copy link
Contributor

pybind/numpy scalars are a bit special: there are zero-dimensional and I think I had to hack a little for them to work.

Yep, I saw this in the C++ side of the failing functions

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...

It looks to me like the errors happen before ADIOS1 even has the chance to do anything?

ERROR: testParticlePatches (API.APITest.APITest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1643, in testParticlePatches
    self.backend_particle_patches(ext)
  File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1591, in backend_particle_patches
    e.particle_patches["numParticles"][SCALAR].store(0, np.uint64(10))
RuntimeError: store: Only scalar values supported!

@franzpoeschel
Copy link
Contributor

First error:

2022-02-18T21:28:07.5858113Z ======================================================================
2022-02-18T21:28:07.5858385Z ERROR: testConstantRecords (API.APITest.APITest)
2022-02-18T21:28:07.5858839Z ----------------------------------------------------------------------
2022-02-18T21:28:07.5859148Z Traceback (most recent call last):
2022-02-18T21:28:07.5859803Z   File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 570, in testConstantRecords
2022-02-18T21:28:07.5860213Z     self.makeConstantRoundTrip(ext)
2022-02-18T21:28:07.5860750Z   File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 427, in makeConstantRoundTrip
2022-02-18T21:28:07.5861164Z     ms["int16"][SCALAR].make_constant(np.int16(234))
2022-02-18T21:28:07.5861504Z RuntimeError: make_constant: Only scalar values supported! (found 2values)

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:

2022-02-18T21:28:07.5836071Z testConstantRecords (API.APITest.APITest) ... Buffer has dimensionality: 1
2022-02-18T21:28:07.5836391Z Extent of dimensionality 0: 2
2022-02-18T21:28:07.5836613Z ERROR

Raw log: https://pipelines.actions.githubusercontent.com/1ySCTQQQJyZRSB9lxu8ROYabgAQhsYw84NgJV467yhBP5Zcqdx/_apis/pipelines/1/runs/9483/signedlogcontent/6?urlExpires=2022-02-28T10%3A48%3A39.2682290Z&urlSigningMethod=HMACV1&urlSignature=Ir0ZKU4%2B0Y%2FW6GqAcfdsg2p5EDvsy5W6Dixw3L84di0%3D

@franzpoeschel
Copy link
Contributor

Second error:

2022-02-18T21:28:07.5861820Z ======================================================================
2022-02-18T21:28:07.5862125Z ERROR: testParticlePatches (API.APITest.APITest)
2022-02-18T21:28:07.5862561Z ----------------------------------------------------------------------
2022-02-18T21:28:07.5862876Z Traceback (most recent call last):
2022-02-18T21:28:07.5863412Z   File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1643, in testParticlePatches
2022-02-18T21:28:07.5863788Z     self.backend_particle_patches(ext)
2022-02-18T21:28:07.5864332Z   File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 1591, in backend_particle_patches
2022-02-18T21:28:07.5864766Z     e.particle_patches["numParticles"][SCALAR].store(0, np.uint64(10))
2022-02-18T21:28:07.5865127Z RuntimeError: store: Only scalar values supported! (found 8values)

Similar behavior:

2022-02-18T21:28:07.5852527Z testParticlePatches (API.APITest.APITest) ... Buffer has dimensionality: 1
2022-02-18T21:28:07.5852846Z Extent of dimensionality 0: 8

@franzpoeschel
Copy link
Contributor

Third error:

2022-02-18T21:28:07.5865436Z ======================================================================
2022-02-18T21:28:07.5865707Z FAIL: testAttributes (API.APITest.APITest)
2022-02-18T21:28:07.5866229Z ----------------------------------------------------------------------
2022-02-18T21:28:07.5866540Z Traceback (most recent call last):
2022-02-18T21:28:07.5867070Z   File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 384, in testAttributes
2022-02-18T21:28:07.5867460Z     self.attributeRoundTrip(ext)
2022-02-18T21:28:07.5867976Z   File "/home/runner/work/openPMD-api/openPMD-api/test/python/unittest/API/APITest.py", line 277, in attributeRoundTrip
2022-02-18T21:28:07.5868476Z     self.assertEqual(series.get_attribute("int16"), 234)
2022-02-18T21:28:07.5868747Z AssertionError: [234, 0] != 234

Error is:

2022-02-18T21:28:07.5827561Z testAttributes (API.APITest.APITest) ... Attribute 'char' has type: STRING
2022-02-18T21:28:07.5828770Z  and value: c
2022-02-18T21:28:07.5829312Z Attribute 'pystring' has type: STRING
2022-02-18T21:28:07.5829738Z  and value: howdy!
2022-02-18T21:28:07.5830225Z Attribute 'pystring2' has type: STRING
2022-02-18T21:28:07.5830611Z  and value: howdy, too!
2022-02-18T21:28:07.5831006Z Attribute 'pystring3' has type: VEC_UCHAR
2022-02-18T21:28:07.5831436Z  and value: [h, o, w, d, y, ,,  , a, g, a, i, n, !]
2022-02-18T21:28:07.5832208Z Attribute 'pyint' has type: UCHAR
2022-02-18T21:28:07.5832768Z  and value: 
2022-02-18T21:28:07.5833076Z Attribute 'pyfloat' has type: DOUBLE
2022-02-18T21:28:07.5833566Z  and value: 3.1416
2022-02-18T21:28:07.5833869Z Attribute 'pybool' has type: BOOL
2022-02-18T21:28:07.5834106Z  and value: 0
2022-02-18T21:28:07.5834399Z Attribute 'int16' has type: VEC_UCHAR
2022-02-18T21:28:07.5834720Z  and value: [�, 
2022-02-18T21:28:07.5834923Z ]
2022-02-18T21:28:07.5835086Z FAIL

@franzpoeschel
Copy link
Contributor

franzpoeschel commented Feb 28, 2022

Summing up the above three messages: pybind11 seems to represent scalar ints as vector of uchar according to the integer's size??
I'm relatively certain that this has nothing to do with ADIOS1.
I think you have more experience with pybind11 datatypes, so I'll wait for your feedback before opening an issue.

@ax3l ax3l added this to the 0.14.6 milestone May 25, 2022
@ax3l ax3l modified the milestones: 0.14.6, 0.14.7 Jun 8, 2022
@ax3l ax3l modified the milestones: 0.14.7, 0.14.6 Apr 3, 2023
@ax3l ax3l modified the milestones: 0.14.6, 0.15.4 Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants