Skip to content

PYBIND11_FINDPYTHON=ON#1684

Merged
ax3l merged 1 commit intoopenPMD:devfrom
ax3l:fix-pybind11-python
Oct 11, 2024
Merged

PYBIND11_FINDPYTHON=ON#1684
ax3l merged 1 commit intoopenPMD:devfrom
ax3l:fix-pybind11-python

Conversation

@ax3l
Copy link
Member

@ax3l ax3l commented Oct 11, 2024 •
edited
Loading

@ax3l ax3l added this to the 0.16.1 milestone Oct 11, 2024
@ax3l ax3l requested a review from franzpoeschel October 11, 2024 16:41
@franzpoeschel
Copy link
Contributor

franzpoeschel commented Oct 11, 2024 •
edited
Loading

Should we add this logic to our superbuilds in CMake? Set PYTHON_EXECUTABLE before adding pybind11 if it is not yet set?

@ax3l
Copy link
Member Author

ax3l commented Oct 11, 2024

Possibly, yes!

We should set the executable that was found here:
https://github.com/openPMD/openPMD-api/blob/0.16.0/cmake/dependencies/pybind11.cmake#L80-L86

So a simple

set(PYTHON_EXECUTABLE ${Python_EXECUTABLE})

might make it already more robust.

This was referenced Oct 11, 2024
Merged
Merged
Merged
@ax3l ax3l changed the title setup.py: Hint pybind11 FindPythonLibsNew PYBIND11_FINDPYTHON=ON Oct 11, 2024
@ax3l ax3l force-pushed the fix-pybind11-python branch from 4be17cc to e8c6f5c Compare October 11, 2024 17:56
@ax3l
Copy link
Member Author

ax3l commented Oct 11, 2024

Ok, so if we set PYBIND11_FINDPYTHON=ON
https://pybind11.readthedocs.io/en/stable/compiling.html#modules-with-cmake

Then pybind11 will call find_package(Python ...) again, which will see that we already called it (default find_package behavior), use our cached results and rely on the new logic
https://github.com/pybind/pybind11/blob/v2.13.6/tools/pybind11Common.cmake#L215-L223

Well, not obvious from the docs, but ok.

@ax3l ax3l force-pushed the fix-pybind11-python branch from e8c6f5c to 9bc591f Compare October 11, 2024 17:59
@ax3l
Copy link
Member Author

ax3l commented Oct 11, 2024 •
edited
Loading

MSVC: calling setup.py with a python 3.9 now finds newest on the system first (outside of setup-python, venv and conda envs):

# python3.exe is c:\hostedtoolcache\windows\python\3.9.13\x64\python3.exe
$ python3.exe -m pip wheel -v .
...
  -- Found Python: C:/hostedtoolcache/windows/Python/3.12.7/x64/python3.exe (found suitable version "3.12.7", minimum required is "3.7.0") found components: Interpreter Development.Module
...

Which hints do we want to set in setup.py to make sure the calling Python process is exactly the Python that is used? (Note: Python_EXECUTABLE is an overwritable output, not an input. Setting it can still mismatch libs, etc.)

MSVC: NumPy requires newer Python:

  Link requires a different Python (3.9.13 not in: '>=3.10'): https://files.pythonhosted.org/packages/d1/0f/8d2b5ebb01dc49d20ae0a282d6baff7202b7bf0df8acdd4a6abeffe98070/numpy-2.1.0rc1.tar.gz (from https://pypi.org/simple/numpy/) (requires-python:>=3.10)

@ax3l ax3l force-pushed the fix-pybind11-python branch 2 times, most recently from de3b2a5 to 7c0972a Compare October 11, 2024 18:36
@ax3l
Copy link
Member Author

ax3l commented Oct 11, 2024

Found a way to hint location and, if multiple versions are installed, version:

@ax3l ax3l force-pushed the fix-pybind11-python branch from 7c0972a to 4ce16ce Compare October 11, 2024 18:38
@ax3l
PYBIND11_FINDPYTHON=ON
22c34fc 
Reuse our `find_package(Python ...)` call and use new CMake logic in
pybind11.
https://pybind11.readthedocs.io/en/stable/compiling.html#modules-with-cmake
https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-version-selection

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>
@ax3l ax3l force-pushed the fix-pybind11-python branch from 4ce16ce to 22c34fc Compare October 11, 2024 18:54
@ax3l ax3l mentioned this pull request Oct 11, 2024
Merged
2 tasks
@ax3l
Copy link
Member Author

ax3l commented Oct 11, 2024

Ok, that should do the trick :)

@ax3l
Copy link
Member Author

ax3l commented Oct 11, 2024

Equivalent PRs to:

all pass :)

@ax3l ax3l merged commit 93df9d5 into openPMD:dev Oct 11, 2024
@ax3l ax3l deleted the fix-pybind11-python branch October 11, 2024 21:05
franzpoeschel added a commit to franzpoeschel/openPMD-api that referenced this pull request Nov 12, 2024
@franzpoeschel
Apply patch for openPMD#1684: PYBIND11_FINDPYTHON=ON
6874639
franzpoeschel added a commit that referenced this pull request Nov 12, 2024
@franzpoeschel
Apply patch for #1684: PYBIND11_FINDPYTHON=ON (#1695)
378b7c5
franzpoeschel pushed a commit to ax3l/openPMD-api that referenced this pull request Dec 20, 2024
@ax3l @franzpoeschel
PYBIND11_FINDPYTHON=ON (openPMD#1684)
9a89862 
Reuse our `find_package(Python ...)` call and use new CMake logic in
pybind11.
https://pybind11.readthedocs.io/en/stable/compiling.html#modules-with-cmake
https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-version-selection

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>
ax3l added a commit that referenced this pull request Jan 16, 2025
@ax3l @franzpoeschel
Release: 0.16.1 (#1676)
3a60e77 
* Release: 0.16.1

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Fix new_version.py

* Update version in windows.yml

* Reformat with yapf3

* Fix greedy Regex parsing

* Changelog: Add PRs #1681 #1679 #1678

* Fix: CMake Internal Control (#1678)

The `openPMD_USE_INTERNAL_*` options are for general internal/external
control. They were accidentally overwritten by new tar flags
and sometimes incomplete.

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Revert "CMake Tar: TOML11 3.7.1 (#1673)" (#1679)

This reverts commit aea4b72.

* Replace deprecated makeSuite with loadTestsFromTestCase (#1681)

Removed with Python 3.13

* Doc: Fix Shipped Internally (#1682)

Downloaded now.

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* `PYBIND11_FINDPYTHON=ON` (#1684)

Reuse our `find_package(Python ...)` call and use new CMake logic in
pybind11.
https://pybind11.readthedocs.io/en/stable/compiling.html#modules-with-cmake
https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-version-selection

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Fix ICX build (#1690)

Error was:
 /home/runner/work/openPMD-api/openPMD-api/include/openPMD/backend/Container.hpp:262:32:
error: no member named 'm_container' in 'Container<T, T_key,
T_container>'
  262 |         container().swap(other.m_container);

* Fix Leftover ADIOS1 Mentions (#1683)

* Fix Leftover ADIOS1 Mentions

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>

* Remove ADIOS1 from examples

---------

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: Franz Pöschel <franz.poeschel@gmail.com>

* Explicitly specify ADIOS2 components in openPMDConfig.cmake (#1693)

* Explicitly specify ADIOS2 components in openPMDConfig.cmake

When including openPMD in downstream CMake and that CMake script
contains Fortran, ADIOS2 will automatically detect that. But since we
do not reexport the ADIOS2 headers, we only need the components that
openPMD explicitly uses: CXX and MPI.

* Same in CMakeLists.txt

* Remove mpirun_workaround.sh (#1698)

Seems to no longer be necessary in the affected CI runs

* Properly check for empty HDF5_VERSION in CMake (#1702)

The old test apparently invoked some weird CMake legacy behavior.
This should make the pinned HDF5 Conda version unnecessary (#1701) since
the version with missing HDF5_VERSION is correctly dealt with.

* HDF5: Delete and re-create attribute when overwriting with diff. type (#1697)

* HDF5: Delete and re-create attribute when overwriting with diff. type

* WIP: Change datatype in test

Somehow truncation test is broken by this

* Add forgotten H5Aclose() call

* TOUCH IOTask: Avoid setting files as dirty in non-write modes (#1704)

* Bugfix: Consider dirty upon touch() only in write modes

* Add error check to avoid writing in read-only mode

* Changelog

* Use ULONG for particle patches

* Upgrade appleclang14 build to MacOS 13

> The macOS-12 environment is deprecated, consider switching to macOS-13,
macOS-14 (macos-latest) or macOS-15. For more details, see
actions/runner-images#10721

* Update Changelog

* Release Date

---------

Signed-off-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: Franz Pöschel <franz.poeschel@gmail.com>
franzpoeschel added a commit to franzpoeschel/openPMD-api that referenced this pull request Feb 5, 2025
@franzpoeschel
Remove venv Python workarounds in our CI workflows
5a23b89 
Those workarounds were probably necessary only due to the build system
issue fixed with openPMD#1684, so we should try removing them again.
@franzpoeschel franzpoeschel mentioned this pull request Feb 5, 2025
Merged
franzpoeschel added a commit that referenced this pull request Feb 10, 2025
@franzpoeschel
Remove venv Python workarounds in our CI workflows (#1717)
cb7ffe1 
* Remove venv Python workarounds in our CI workflows

Those workarounds were probably necessary only due to the build system
issue fixed with #1684, so we should try removing them again.

* Use correct Python versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@franzpoeschel franzpoeschel Awaiting requested review from franzpoeschel

Assignees

@franzpoeschel franzpoeschel

Labels

affects latest release bug frontend: Python3 install

Projects

None yet

Milestone

0.16.1

Development

Successfully merging this pull request may close these issues.

2 participants

@ax3l @franzpoeschel