Skip to content

Conversation

@nicholasjng
Copy link
Contributor

This seems to reduce binding sizes even further, with a wheel size of 175KB on my local machine (macOS 13.4.1).

"src/nb_internals.cpp",
"src/nb_internals.h",
"src/nb_ndarray.cpp",
"src/nb_static_property.cpp",
Copy link
Member

Choose a reason for hiding this comment

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

should this be sourced as an external library from somewhere (github release tarball?) so we don't need to have a parallel build of the library maintained here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be used as a build-time CMake dependency via scikit-build, but this requires changing the package build to use scikit-build - that could be an avenue.

Otherwise, there is an open JAX PR that ports some existing pybind11 code to nanobind - there they have a leaner build file, but still explicit configs (I think that is sensible too tbh)

In the end, however, we are pulling straight from github in Bazel too, so this might already be what you intended? (The build file is necessary to mark it as a Bazel dep)

Copy link
Member

Choose a reason for hiding this comment

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

i thought similar to how we depend on googletest, we could depend on nanobind. but maybe it's a different type of dependency if we don't directly link to it? or maybe nanobind doesn't support bazel directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, nanobind only supports CMake. So I'm afraid we need the full build file setup regardless. But at least the link above shows that it can still be simplified, especially if we don't particularly care about pinning the submodules.

@nicholasjng nicholasjng force-pushed the bump-nanobind-to-14 branch from ad35749 to f375f06 Compare July 10, 2023 11:26
This seems to reduce binding sizes even further, with a wheel size of
175KB on my local machine (macOS 13.4.1).
@nicholasjng nicholasjng force-pushed the bump-nanobind-to-14 branch from f375f06 to 99698d7 Compare July 10, 2023 18:05
@dmah42 dmah42 merged commit cb39b71 into google:main Jul 11, 2023
@nicholasjng nicholasjng deleted the bump-nanobind-to-14 branch April 15, 2024 16:46
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