Skip to content
This repository was archived by the owner on Dec 19, 2025. It is now read-only.

ENH: set PyBind11 version to 3.0#68

Closed
kennykos wants to merge 23 commits intokokkos:mainfrom
kennykos:pybind11v3.0
Closed

ENH: set PyBind11 version to 3.0#68
kennykos wants to merge 23 commits intokokkos:mainfrom
kennykos:pybind11v3.0

Conversation

@kennykos
Copy link
Copy Markdown

@kennykos kennykos commented Oct 9, 2025

No description provided.

@kennykos
Copy link
Copy Markdown
Author

kennykos commented Oct 9, 2025

Closes #67

Copy link
Copy Markdown
Contributor

@JBludau JBludau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me.

@kennykos
Copy link
Copy Markdown
Author

kennykos commented Oct 9, 2025

@JBludau if you (or another maintainer) approve the workflows then I will be able to merge.

@JBludau
Copy link
Copy Markdown
Contributor

JBludau commented Oct 9, 2025

I think we should also bump the minimum python version. Any suggestions?

I think 3.10 would be a reasonable minimum.
And while we are at it, we should think about bumping Kokkos to 4.7.01

@kennykos
Copy link
Copy Markdown
Author

kennykos commented Oct 9, 2025

@JBludau I've updated the necessary workflows as per https://devguide.python.org/versions/, anything else I need to update? Also, I think the workflows need to be approved again.

@JBludau
Copy link
Copy Markdown
Contributor

JBludau commented Oct 9, 2025

@JBludau I've updated the necessary workflows as per https://devguide.python.org/versions/, anything else I need to update? Also, I think the workflows need to be approved again.

The versions are specified in multiple places, which should be unified. Also there is logic for python versions in pypy. I will take a look at the workflows later

@kennykos
Copy link
Copy Markdown
Author

kennykos commented Oct 9, 2025

@JBludau Ok I updated Kokkos and also changed the python versions everywhere (I was being lazy before and missed some locations). Hopefully the tests pass now!

Copy link
Copy Markdown
Contributor

@JBludau JBludau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can not just drop the formatting. Also there is multiple dead branches and ifs in the workflows now

@kennykos
Copy link
Copy Markdown
Author

kennykos commented Oct 9, 2025

We can not just drop the formatting. Also there is multiple dead branches and ifs in the workflows now

Sorry about that. All the stale if branches are now gone and the formatting is back, let's hope everything works 🤞

@kennykos kennykos requested a review from JBludau October 9, 2025 19:53
Copy link
Copy Markdown
Contributor

@JBludau JBludau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kokkos now requires CMake>3.22

Comment thread .github/workflows/python-package.yml Outdated
Comment thread .github/workflows/python-package.yml Outdated
Comment thread .github/workflows/python-package.yml Outdated
Comment thread .github/workflows/linux-ci.yml Outdated
kennykos and others added 4 commits October 9, 2025 13:57
Co-authored-by: JBludau <104908666+JBludau@users.noreply.github.com>
Co-authored-by: JBludau <104908666+JBludau@users.noreply.github.com>
@kennykos kennykos requested a review from JBludau October 9, 2025 20:05
@JBludau
Copy link
Copy Markdown
Contributor

JBludau commented Oct 9, 2025

see actions/runner#1989

@JBludau
Copy link
Copy Markdown
Contributor

JBludau commented Oct 9, 2025

I guess CMake should be 3.31.6

@kennykos
Copy link
Copy Markdown
Author

@JBludau my thoughts handle the /home/runner/work/pykokkos-base/pykokkos-base/include/pools.hpp:71:12: error: ‘class Kokkos::Random_XorShift64_Pool<Kokkos::Serial>’ has no member named ‘init’ error is to just do _p(_seed, _num_states);?
It seems like that should work based off the Kokkos documentation https://kokkos.org/kokkos-core-wiki/API/algorithms/Random-Number.html. Do you have any thoughts?

@kennykos
Copy link
Copy Markdown
Author

@JBludau is also looks like the "setuptools < 60.0.0", requirement is causing the AttributeError: module 'pkgutil' has no attribute 'ImpImporter'. Did you mean: 'zipimporter'? error (see https://setuptools.pypa.io/en/latest/). Do you have any idea why we have the setup-tools restriction? Either way, I'll debug these installs locally.

@tylerjereddy
Copy link
Copy Markdown
Contributor

Do you have any idea why we have the setup-tools restriction?

setuptools gets frequent updates that can cause breaks in builds, was probably done for that reason, pretty common thing to do. Maybe time to explore bumping it yeah.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants