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: Linux aarch64/arm64 #1517

Merged
merged 11 commits into from
Dec 12, 2023
Merged

CI: Linux aarch64/arm64 #1517

merged 11 commits into from
Dec 12, 2023

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Aug 19, 2023

Add native Linux aarch64/arm64 runners with CircleCI.

Adding CI here first before adding an entry to wheels for building Python (Pip) Wheels.
Note: We already successfully build this architecture on conda-forge.

@ax3l ax3l mentioned this pull request Aug 19, 2023
@ax3l ax3l added this to the 0.15.3 milestone Aug 19, 2023
@ax3l ax3l marked this pull request as draft August 19, 2023 21:58
@ax3l ax3l force-pushed the ci-linux-arm64 branch 3 times, most recently from 10e4901 to e2a59b1 Compare August 19, 2023 22:07
@ax3l ax3l marked this pull request as ready for review August 19, 2023 22:40
@ax3l ax3l force-pushed the ci-linux-arm64 branch 2 times, most recently from 54953cd to 4364b59 Compare August 19, 2023 23:07
@ax3l ax3l requested a review from franzpoeschel August 19, 2023 23:07
@ax3l ax3l force-pushed the ci-linux-arm64 branch 9 times, most recently from 40bcdca to 73c84c3 Compare August 20, 2023 00:01
@ax3l
Copy link
Member Author

ax3l commented Aug 20, 2023

Huh:

======================================================================
ERROR: testAttributes (API.APITest.APITest.testAttributes)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/circleci/linux-aarch64-wheels/test/python/unittest/API/APITest.py", line 381, in testAttributes
    self.attributeRoundTrip(ext)
  File "/home/circleci/linux-aarch64-wheels/test/python/unittest/API/APITest.py", line 267, in attributeRoundTrip
    self.assertEqual(bytes(series.get_attribute("pystring3")),
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'str' object cannot be interpreted as an integer

and in CLI.pipe.py:

    dest.set_attribute(key, attr, attr_type)
    run_pipe.run()
RuntimeError: Unable to cast Python instance of type <class 'str'> to C++ type '?' (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)

@ax3l
Copy link
Member Author

ax3l commented Aug 20, 2023

Time to fix our long-standing ODR issue...: #1521

Update: merged & rebased (good), but still the same error here.

@ax3l ax3l force-pushed the ci-linux-arm64 branch 4 times, most recently from fd17aca to d951f12 Compare August 20, 2023 04:06
@ax3l ax3l modified the milestones: 0.15.3, 0.15.4 Oct 6, 2023
.circleci/config.yml Outdated Show resolved Hide resolved
@ax3l ax3l added the machine/system machine & HPC system specific issues label Nov 7, 2023
@franzpoeschel
Copy link
Contributor

franzpoeschel commented Nov 9, 2023

For the error in openpmd-pipe, we use a special form of set_attribute that lets us explicitly specify the datatype:

dest.set_attribute(key, attr, attr_type)

This finally runs into this implementation where list and string types are handled separately, since the Numpy types in attribute_dtypes are the underlying types (i.e. char instead of string):

template <>
bool SetAttributeFromObject::call<char>(
    Attributable &attr, std::string const &key, py::object &obj)
{
    if (std::string(py::str(obj.get_type())) == "<class 'list'>")
    {
        using ListChar = std::vector<char>;
        using ListString = std::vector<std::string>;
        try
        {
            return attr.setAttribute<ListString>(key, obj.cast<ListString>());
        }
        catch (const py::cast_error &)
        {
            return attr.setAttribute<ListChar>(key, obj.cast<ListChar>());
        }
    }
    else if (std::string(py::str(obj.get_type())) == "<class 'str'>")
    {
        return attr.setAttribute<std::string>(key, obj.cast<std::string>());
    }
    else
    {
        return attr.setAttribute<char>(key, obj.cast<char>());
    }
}

It looks like on Arm64, <class 'str'> does not work to identify strings? Maybe there's a better way.
The other error from APITest.py looks similar, but not the same.

@franzpoeschel
Copy link
Contributor

I pushed something that might fix the openpmd-pipe problem. Aside from this PR, we should add a test that tests openpmd-pipe against the result of dtype_test, there were some broken char types.

@franzpoeschel franzpoeschel force-pushed the ci-linux-arm64 branch 4 times, most recently from ac9b2dc to 8a14a72 Compare November 10, 2023 08:49
@franzpoeschel
Copy link
Contributor

This is the problem:

Setting attribute 'date' with type <class 'str'>' and requested type int8, i.e. openPMD type SCHAR
        'char' is unsigned on the platform

Char is represented as unsigned char, but Python reports signed char as type for strings. This might be since the datasets are the sample datasets written on another platform?

@franzpoeschel franzpoeschel force-pushed the ci-linux-arm64 branch 2 times, most recently from ec96caa to 4e8ec42 Compare November 21, 2023 10:18
@franzpoeschel
Copy link
Contributor

The openpmd-pipe thing seems to be fixed now. The remaining error is:

======================================================================
ERROR: testAttributes (API.APITest.APITest.testAttributes)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/circleci/linux-aarch64-wheels/test/python/unittest/API/APITest.py", line 381, in testAttributes
    self.attributeRoundTrip(ext)
  File "/home/circleci/linux-aarch64-wheels/test/python/unittest/API/APITest.py", line 267, in attributeRoundTrip
    self.assertEqual(bytes(series.get_attribute("pystring3")),
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'str' object cannot be interpreted as an integer

Reminder: This test on an AMD64 system:

>>> import openpmd_api as io                                                                                          
>>> s = io.Series("asdf.json", io.Access.create)                                                                      
>>> s.set_attribute("pystring3", b"howdy, again!")                                                                    
False                                                                                                                 
>>> s.close()     

Results in:

{
  "attributes": {
    "...": "...",
    "pystring3": {
      "datatype": "VEC_UCHAR",
      "value": [
        104,
        111,
        119,
        100,
        121,
        44,
        32,
        97,
        103,
        97,
        105,
        110,
        33
      ]
    },
   "...": "...",
  },
  "data": {}
}

Arm64 somehow seems to make a string out of this again. I've pushed a printf debugging commit.

@franzpoeschel franzpoeschel force-pushed the ci-linux-arm64 branch 2 times, most recently from 54a5dbd to acc5ea8 Compare November 28, 2023 12:17
@franzpoeschel
Copy link
Contributor

The remaining error is HDF5-specific:

BACKEND: json
DEBUG: pystring3='[104, 111, 119, 100, 121, 44, 32, 97, 103, 97, 105, 110, 33]' of type '<class 'list'>'
openPMD series: unittest_py_API
openPMD standard: 1.1.0
openPMD extensions: 0

number of iterations: 0

number of meshes: 0

number of particle species: 0

Series.set_software_version is deprecated. Set the version with the second argument of Series.set_software
BACKEND: toml
DEBUG: pystring3='[104, 111, 119, 100, 121, 44, 32, 97, 103, 97, 105, 110, 33]' of type '<class 'list'>'
openPMD series: unittest_py_API
openPMD standard: 1.1.0
openPMD extensions: 0

number of iterations: 0

number of meshes: 0

number of particle species: 0

Series.set_software_version is deprecated. Set the version with the second argument of Series.set_software
BACKEND: bp
DEBUG: pystring3='[104, 111, 119, 100, 121, 44, 32, 97, 103, 97, 105, 110, 33]' of type '<class 'list'>'
openPMD series: unittest_py_API
openPMD standard: 1.1.0
openPMD extensions: 0

number of iterations: 0

number of meshes: 0

number of particle species: 0

Series.set_software_version is deprecated. Set the version with the second argument of Series.set_software
BACKEND: bp4
DEBUG: pystring3='[104, 111, 119, 100, 121, 44, 32, 97, 103, 97, 105, 110, 33]' of type '<class 'list'>'
openPMD series: unittest_py_API
openPMD standard: 1.1.0
openPMD extensions: 0

number of iterations: 0

number of meshes: 0

number of particle species: 0

Series.set_software_version is deprecated. Set the version with the second argument of Series.set_software
BACKEND: h5
DEBUG: pystring3='['h', 'o', 'w', 'd', 'y', ',', ' ', 'a', 'g', 'a', 'i', 'n', '!']' of type '<class 'list'>'

This is not really a surprise: HDF5 does not have an actual char type, so on a platform with unsigned chars, this will be returned as char and not as unsigned char.

TLDR: This is probably not an error in the openPMD-api, but just a hardcoded test that does not work on this platform.

@franzpoeschel
Copy link
Contributor

New problem: series.set_attribute("pyint", 13) writes a char type. This bug is not introduced, by uncovered by this PR. Adding a workaround for now, proper fix should come along with a rework of set_attribute() type detection.

@franzpoeschel
Copy link
Contributor

All issues fixed now. I'll push another commit that brings some further fixes, but might also bring regressions. If the CI shows regressions, I'll revert it.

@franzpoeschel
Copy link
Contributor

The remaining failing tests are the appleclang14 tests that are currently failing everywhere. However one of them seems to be an Arm64 test, so we might want to wait for #1565 regardless (I think I've found a workaround there).

@franzpoeschel
Copy link
Contributor

@ax3l now passing

@ax3l
Copy link
Member Author

ax3l commented Dec 12, 2023

Thank you for the help 🙏 🎉

@ax3l ax3l merged commit 55af0db into dev Dec 12, 2023
52 checks passed
@ax3l ax3l deleted the ci-linux-arm64 branch December 12, 2023 08:10
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.

2 participants