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.

Sign up for GitHub

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

Jump to bottom

Update nanobind to v2.0 #1790

Open
hawkinsp opened this issue May 24, 2024 · 2 comments
Open

Update nanobind to v2.0 #1790

hawkinsp opened this issue May 24, 2024 · 2 comments

Comments

@hawkinsp
Copy link
Member

nanobind v2.0 was released today; google/benchmark should upgrade.

A change something like the following should work:

--- a/third_party/benchmark/bindings/python/google_benchmark/benchmark.cc
+++ b/third_party/benchmark/bindings/python/google_benchmark/benchmark.cc
@@ -109,7 +109,7 @@ NB_MODULE(_benchmark, m) {
   using benchmark::Counter;
   nb::class_<Counter> py_counter(m, "Counter");

-  nb::enum_<Counter::Flags>(py_counter, "Flags")
+  nb::enum_<Counter::Flags>(py_counter, "Flags", nb::is_arithmetic())
       .value("kDefaults", Counter::Flags::kDefaults)
       .value("kIsRate", Counter::Flags::kIsRate)
       .value("kAvgThreads", Counter::Flags::kAvgThreads)
@@ -121,7 +121,7 @@ NB_MODULE(_benchmark, m) {
       .value("kAvgIterationsRate", Counter::Flags::kAvgIterationsRate)
       .value("kInvert", Counter::Flags::kInvert)
       .export_values()
-      .def(nb::self | nb::self);
+      .def("__or__", [](Counter::Flags a, Counter::Flags b) { return a | b; });

   nb::enum_<Counter::OneK>(py_counter, "OneK")
       .value("kIs1000", Counter::OneK::kIs1000)

although I don't have time right now to figure out the bazel build updates, etc.
@nicholasjng
Copy link
Contributor

nicholasjng commented May 24, 2024 •
edited
Loading

This will require a new release of nanobind-bazel on the BCR, since that's being used to generate the extension.

I haven't gotten around to implement stubgen yet to generate stubs during the Bazel build, but that should not very difficult (albeit I'm not a Bazel expert). I'll find some time to get it done soon.

EDIT: nvm, you already posted an issue. thanks!

@varshneydevansh
Copy link
Contributor

I am trying to do this and did the version change in the MODULE.build file.

 base  devansh   1790_update_nanobind -  bazel build                               [24/05/24| 6:37PM]
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
ERROR: Error computing the main repository mapping: in module dependency chain <root> -> [email protected]: module not found in registries: [email protected]
Computing main repo mapping: 
    Fetching https://bcr.bazel.build/modules/rules_python/0.31.0/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/googletest/1.12.1/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/platforms/0.0.7/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/protobuf/3.19.6/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/apple_support/1.5.0/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/rules_java/7.4.0/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/buildozer/6.4.0.2/MODULE.bazel
    Fetching https://bcr.bazel.build/modules/rules_foreign_cc/0.10.1/MODULE.bazel

Now I understood how it's dependent on the https://github.com/nicholasjng/nanobind-bazel

So, now I have to implement stubgen to generate stubs in the nanobind-bazel as told by @nicholasjng ?

@nicholasjng nicholasjng mentioned this issue May 27, 2024
Merged
nicholasjng added a commit to nicholasjng/benchmark that referenced this issue May 29, 2024
@nicholasjng
Upgrade google/benchmark to use nanobind-bazel v2.0.0
7eb18fa 
This enables binding extension builds with nanobind@v2.

Due to some backwards-incompatible changes in the treatment of enums,
`Counter::Flags` (a flag-based enum) needs to be explicitly marked as
arithmetic; setting flags is done as before with the logical "or"
operator (now bound like any other class method as `__or__()`, the
Python logical or operator slot).

The bindings changes were suggested by @hawkinsp in the corresponding
issue on the repo, see google#1790.
@nicholasjng nicholasjng mentioned this issue May 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade google/benchmark to use nanobind-bazel v2.0.0 nicholasjng/benchmark
3 participants
@hawkinsp @varshneydevansh @nicholasjng