optree 0.12.x requires pybind 2.12 or greater

icanhasmath commented 1 month ago

Required prerequisites

[X] I have read the documentation https://optree.readthedocs.io.
[X] I have searched the Issue Tracker that this hasn't already been reported. (comment there if it has.)

What version of OpTree are you using?

0.12.x

System information

Python 3.10, Building from source on Linux x86_64, and Windows amd64.

Problem description

The compilation fails with older versions of pybind11.

Reproducible example code

Standard Compilation from source of Optree 1.12.x with pybind 2.11.x.

Traceback

At compile time the compiler throws the same errors repeatedly from `optree-0.12.1\include\utils.h`

On Linux:
/build/optree-0.12.1/include/utils.h: In function ‘PyObject* Py_ID_optree()’:
/build/optree-0.12.1/include/utils.h:113:9: error: ‘PYBIND11_CONSTINIT’ was not declared in this scope; did you mean ‘PYBIND11_CONCAT’?
  113 |         PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<PyObject*> storage; \
      |         ^~~~~~~~~~~~~~~~~~
/build/optree-0.12.1/include/utils.h:128:1: note: in expansion of macro ‘Py_Declare_ID’
  128 | Py_Declare_ID(optree);
      | ^~~~~~~~~~~~~
/build/optree-0.12.1/include/utils.h:114:16: error: ‘storage’ was not declared in this scope
  114 |         return storage                                                                 \
      |                ^~~~~~~
/build/optree-0.12.1/include/utils.h:128:1: note: in expansion of macro ‘Py_Declare_ID’
  128 | Py_Declare_ID(optree);
      | ^~~~~~~~~~~~~
/build/optree-0.12.1/include/utils.h: In function ‘PyObject* Py_ID___main__()’:

On Windows:
error C2039: 'gil_safe_call_once_and_store': is not a member of 'pybind11' [C:\build\optree-0.12.1\build\temp.win-amd64-cpython-310\Release\src\_C.vcxproj]
error C2062: type 'unknown-type' unexpected [C:\build\optree-0.12.1\build\temp.win-amd64-cpython-310\Release\src\_C.vcxproj]
error C2065: 'PYBIND11_CONSTINIT': undeclared identifier [C:\build\optree-0.12.1\build\temp.win-amd64-cpython-310\Release\src\_C.vcxproj]
error C2065: 'storage': undeclared identifier [C:\build\optree-0.12.1\build\temp.win-amd64-cpython-310\Release\src\_C.vcxproj]
error C2144: syntax error: 'int' should be preceded by ';' [C:\build\optree-0.12.1\build\temp.win-amd64-cpython-310\Release\src\_C.vcxproj]
error C7568: argument list missing after assumed function template 'gil_safe_call_once_and_store' [C:\build\optree-0.12.1\build\temp.win-amd64-cpython-310\Release\src\_C.vcxproj]

Expected behavior

Expected that there is a restriction in the pyproject toml to prevent an incompatible selection.

Additional context

You can see my test builds here, I tried a few different combinations to pin down the range: https://platform.activestate.com/ActiveStateBE/optree/releases

icanhasmath commented 1 month ago

I tried to push a branch, but this is the proposed fix:

diff --git a/pyproject.toml b/pyproject.toml
index af0470e..6ad5e5a 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,7 +1,7 @@
 # Package ######################################################################

 [build-system]
-requires = ["setuptools", "pybind11"]
+requires = ["setuptools", "pybind11>=2.12.0"]
 build-backend = "setuptools.build_meta"

 [project]

XuehaiPan commented 1 month ago

Hi @icanhasmath, have you tried the main branch?

pip3 install git+https://github.com/metaopt/optree.git

icanhasmath commented 1 month ago

Pip3 install works just fine - there is no problem there.

This case arises building with python3 directly from source, in an isolated environment, with an older version of pybind11.

XuehaiPan commented 1 month ago

I think PR #151 resolved this.

151

Previously, cmake may look for system pybind11 rather than the one installed inside the virtual environment.

icanhasmath commented 1 month ago

Apologies - I should probably have called this an enhancement rather than a bug. There is nothing wrong with the standard build process - the request here is to clarify the pybind11 specification, purely for accuracy. I'm happy to close this issue if you want to leave it unpinned.

metaopt / optree