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

Update nanobind to v2 #1777

Merged
merged 7 commits into from
Jun 13, 2024
Merged

Update nanobind to v2 #1777

merged 7 commits into from
Jun 13, 2024

Conversation

havogt
Copy link
Contributor

@havogt havogt commented May 29, 2024

@havogt
Copy link
Contributor Author

havogt commented May 29, 2024

launch jenkins

@havogt
Copy link
Contributor Author

havogt commented May 29, 2024

Because of this change wjakob/nanobind#377 our test setup is now broken. We have to run a test from Python.

@havogt
Copy link
Contributor Author

havogt commented May 30, 2024

launch jenkins

@havogt havogt requested a review from fthaler May 30, 2024 11:51
@havogt
Copy link
Contributor Author

havogt commented May 30, 2024

launch jenkins

tests/unit_tests/storage/adapter/test_nanobind_adapter.cpp Outdated Show resolved Hide resolved
tests/unit_tests/storage/adapter/test_nanobind_adapter.cpp Outdated Show resolved Hide resolved
template <std::size_t... Values>
using stride_spec = std::index_sequence<Values...>;
// Use `-1` for dynamic stride, use an integral value for static stride.
template <ssize_t... Values>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we use int for indices internally at quite a few places in GridTools, so maybe this would also be good enough here (this might even have a performance impact).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually used the wrong ssize_t. It's actually a nb type defined here https://github.com/wjakob/nanobind/blob/c5ae2a36a704c1c62da99a81fad919261a3e9848/include/nanobind/nb_traits.h#L11.

I'll use that. We could still convert (runtime) strides to int, but I would do separately and probably should look at performance.

@havogt havogt requested a review from fthaler June 12, 2024 09:07
@havogt
Copy link
Contributor Author

havogt commented Jun 13, 2024

launch jenkins

@havogt
Copy link
Contributor Author

havogt commented Jun 13, 2024

launch jenkins

@havogt havogt merged commit 88e7d91 into GridTools:master Jun 13, 2024
61 checks passed
@havogt havogt deleted the nanobind2 branch June 13, 2024 12:32
havogt added a commit that referenced this pull request Jun 18, 2024
- nb::any is replaced by `nb::ssize_t(-1)`
- Because of a change in `~ndarray`, we need to make sure that Python is initialized in our tests (see wjakob/nanobind#377)
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 this pull request may close these issues.

2 participants