Skip to content

Conversation

@janbridley
Copy link
Contributor

@janbridley janbridley commented Nov 22, 2025

Description

This is a combination rewrite/refactor that aims to (1) replace pybind11 code with nanobind and (2) restructure data types and the code layout to interface better with cymmetry (upcoming library & publication) and new optimizer developments.

The biggest change is a separation of python interfaces from c++ source code -- bindings are consolidated to separate files, and much of the library is now available as headers for use in other tools. The area of the interface is also reduced, with nanobind's ndarray replacing python exports for Vec3 and Quaternion types exported in previous versions. Finally, the data classes (PGOP- and BOOSOPStore) have been removed. These adapters were added in previous versions for performance reasons, but nanobind allows for copy-free array translation that removes this necessity. These changes, combined with a few other optimizations, result in a 400-500% performance increase across the board.

Changed

  • Project now uses C++ 20 (primarily for std::span). This is great for ergonomics and makes a future Eigen3 port much easier (Eigen::Map is very similar)
  • Nanobind exports replace pybind11
  • Optimizers are now header-only
  • Locality code is now header-only
  • Vec3 and Quaternion are now header-only
  • BondOrder is now header-only
  • Metrics and Utils (excluding QlmEval) are now header only
  • pgop.py::BOOSOP code is now in separate file boosop.py.
  • Many std::vector<std::vector<...>> are now vectors of pointers, allowing for copy- and move- free access to python data. Matrix elements are accessed with std::span and cast to statically-allocated types for performance.
  • py::array are now replaced with std::vector or type* pointers
  • Implied rotation matrix type (std::vector<double>) is now typedef RotationMatrix = std::array<double, 9>

Removed

  • PGOPStore
  • BOOSOPStore
  • Unused python bindings (quaternion, vec3, QLMEval, metrics)

Added

  • m_group_sizes class method for PGOP, which stores the size of each group (currently, (group order - 1) * 9). Previous code used vector.size, which requires copies and allocations for both individual elements and entire groups.
  • RotationMatrix std::array wrapper for fast and strongly typed vector rotations
  • -DENABLE_PROFILING flag to allow for easy profiling

Benchmarking

uv pip install . --config-settings=cmake.args="-DENABLE_PROFILING=ON"  --config-settings=cmake.build-type="RelWithDebInfo"

Before this PR

Compute PGOP for mesh of 600 points, computed for an icosahedron (N=12, N_query=1):

--- Benchmarking C2 symmetry ---
  PGOP: 0.9031 ± 0.0057 (mean ± std. dev.)
  Time: 0.59 μs ± 0.01 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

--- Benchmarking D5d symmetry ---
  PGOP: 0.8648 ± 0.0413 (mean ± std. dev.)
  Time: 8.40 μs ± 0.03 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

--- Benchmarking T symmetry ---
  PGOP: 0.8623 ± 0.0286 (mean ± std. dev.)
  Time: 4.91 μs ± 0.02 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

--- Benchmarking Ih symmetry ---
  PGOP: 0.8365 ± 0.0506 (mean ± std. dev.)
  Time: 51.63 μs ± 0.45 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

After this PR

Compute PGOP for mesh of 600 points, computed for an icosahedron (N=12, N_query=1):

--- Benchmarking C2 symmetry ---
  PGOP: 0.9031 ± 0.0057 (mean ± std. dev.)
  Time: 0.18 μs ± 0.01 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

--- Benchmarking D5d symmetry ---
  PGOP: 0.8648 ± 0.0413 (mean ± std. dev.)
  Time: 1.82 μs ± 0.01 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

--- Benchmarking T symmetry ---
  PGOP: 0.8623 ± 0.0286 (mean ± std. dev.)
  Time: 1.09 μs ± 0.01 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

--- Benchmarking Ih symmetry ---
  PGOP: 0.8365 ± 0.0506 (mean ± std. dev.)
  Time: 11.01 μs ± 0.15 per trial(mean ± std. dev. of 10 runs, 50 orientations each)

Motivation and Context

Resolves: #???

How Has This Been Tested?

Checklist:

@janbridley janbridley marked this pull request as ready for review November 25, 2025 20:26
@janbridley janbridley requested a review from DomFijan November 25, 2025 20:26
@janbridley janbridley mentioned this pull request Nov 25, 2025
@janbridley janbridley requested a review from joaander November 25, 2025 20:28
@DomFijan
Copy link
Collaborator

Impressive work!!

@DomFijan
Copy link
Collaborator

DomFijan commented Nov 25, 2025

Can you revert my change to removing specialized windows testing? It seems we might need that back .... If you can't find it in the sea of commits here, I can do that when I get to reviewing this. It seems that rerun failures wasn't the source of the problems after all. We will have to investigate that issue further at some point, on a Windows machine...

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.

3 participants