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

MAINT: python 3.11 support #44

Closed
tylerjereddy opened this issue Aug 30, 2022 · 5 comments · Fixed by #46
Closed

MAINT: python 3.11 support #44

tylerjereddy opened this issue Aug 30, 2022 · 5 comments · Fixed by #46

Comments

@tylerjereddy
Copy link
Contributor

Python 3.11 RC1 is guaranteed to be ABI stable, so it is safe to start thinking/testing for compatibility there.

Currently, I see some pybind11-related failures like this during pykokkos-base build:

| [ 75%] Building CXX object CMakeFiles/libpykokkos-core.dir/Unity/unity_1_cxx.cxx.o
| In file included from /tmp/pykokkos-base/external/pybind11/include/pybind11/cast.h:16,
|                  from /tmp/pykokkos-base/external/pybind11/include/pybind11/attr.h:13,
|                  from /tmp/pykokkos-base/external/pybind11/include/pybind11/pybind11.h:45,
|                  from /tmp/pykokkos-base/include/common.hpp:54,
|                  from /tmp/pykokkos-base/include/traits.hpp:47,
|                  from /tmp/pykokkos-base/_skbuild/linux-x86_64-3.11/cmake-build/src/variants/views.cpp:3,
|                  from /tmp/pykokkos-base/_skbuild/linux-x86_64-3.11/cmake-build/CMakeFiles/libpykokkos-core.dir/Unity/unity_1_cxx.cxx:3:
| /tmp/pykokkos-base/external/pybind11/include/pybind11/detail/type_caster_base.h: In function ‘std::string pybind11::detail::error_string()’:
| /tmp/pykokkos-base/external/pybind11/include/pybind11/detail/type_caster_base.h:448:36: error: invalid use of incomplete type ‘PyFrameObject’ {aka ‘struct _frame’}
|   448 |                 "  " + handle(frame->f_code->co_filename).cast<std::string>() +
|       |                                

If you want to reproduce you can i.e., modify the pykokkos repo CI:

--- a/.github/workflows/main_ci.yml
+++ b/.github/workflows/main_ci.yml
@@ -13,7 +13,7 @@ jobs:
     strategy:
       matrix:
         platform: [ubuntu-latest]
-        python-version: ["3.8", "3.9", "3.10"]
+        python-version: ["3.11.0-rc.1"]
     runs-on: ${{ matrix.platform }}
     steps:
       - uses: actions/checkout@v3

and run act -j test_pykokkos

@jrmadsen
Copy link
Contributor

$20 says that's fixed in 3.11.0-rc2. This has nothing to do with the ABI, it's a header include issue. They aren't going to break every package which uses pybind11 for that.

@tylerjereddy
Copy link
Contributor Author

How old is /tmp/pykokkos-base/external/pybind11 that gets vendored?

My first bet would have been that pybind11 would have shimmed around that by now. I'll try to check that later tonight. Most other large packages I've seen are already passing tests with pybind11 + 3.11, so it is a safe bet that we just need to bump pybind11 I think.

@jrmadsen
Copy link
Contributor

jrmadsen commented Aug 31, 2022

I think I just used the current main or stable branch when I create the repo. No special changes, you probably just need to do a git pull inside the submodule. Looks like it was noted and fixed in Oct 2021:

pybind/pybind11#3367

pybind/pybind11#3368

tylerjereddy added a commit to tylerjereddy/pykokkos that referenced this issue Aug 31, 2022
* Python `3.11` RC1 is ABI stable, so might
as well test against it

* we should be able to remove the custom `pybind11`
submodule checkout once we update upstream per:
kokkos/pykokkos-base#44
@tylerjereddy
Copy link
Contributor Author

Looks all good with 3.11rc1 over in kokkos/pykokkos#74 now. I'll leave this open as a reminder to update the submodule in this repo, then can remove the manual submodule update over there.

@tylerjereddy
Copy link
Contributor Author

I guess we might want a single 3.11rc1 job in this repo as well? I'll try to help resolve the current CI issues here first though, before expanding more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants