Skip to content

Conversation

@petrasvestartas
Copy link
Collaborator

@petrasvestartas petrasvestartas commented Mar 4, 2025

Nanobind is the next generation of pybind11.
The intent is to update compas_cgal wrapping method to nanobind.

How to run it?

Environment

Create or use an environment with compas, compas_viewer and numpy

Build It

To build a project run:
pip install -e .
or verbose:
pip install -e . -v

First time this command runs, it downloads the C++ source code of CGAL, Eigen and Boost.
Any other time you run this command again, only build process is executed.

Run example

compas_cgal/tests/test_meshing_remesh.py
Screenshot 2025-03-04 at 19 48 36

Goal of the pull request

This pull request intends to implement nanobind for:
a) faster C++ compilation times when precompiled headers are used
b) dependency download when pip install -e . is called (no conda dependencies)
c) use CGAL with Boost and get rid of GMP and MPFR dependencies

ToDo:

☑2025-02-27 nanobind hello world works and cgal, boost and eigen is downloaded on the first call of "pip install -e ."
☑2025-02-28 reference headers and wrap one single method
☑2025-02-05 setup precompiled headers for faster development time
☑2025-02-07 Part 1 wrap the rest of current compas_cgal methods
☑ 2025-02-08 Part 2 wrap the rest of current compas_cgal methods
2025-02-10 Documentation.
2025-02-11 test the repository on MAC M and Intel Chips as well as Linux
2025-02-12 Github CI
2025-02-13 discuss the distribution to pypi and conda

@petrasvestartas
Copy link
Collaborator Author

I would like to ask a review of the current stage of the code.
Currently, there is only one method that is closely aligned how pybind11 was written in a nanobind style.

After review and changes, the goal is to implement the rest of the methods.

@tomvanmele
Copy link
Member

so far so good it seems. just a few questions/suggestions:

  • what is the plan with the remaining files of the repo?
  • why is compas.h/cpp renamed to stdafx.h/cpp?
  • i find the "manual profiling" in the tests a bit messy. perhaps the tests can just be about testing, while separate dedicated scripts are used for profiling (ideally with a profiler)

@petrasvestartas
Copy link
Collaborator Author

petrasvestartas commented Mar 4, 2025

Plan for the Remaining Repository Files

Source Files

  • The plan is to migrate all C++ and Python files from the current repository and translate them one by one to Nanobind, keeping the differences as minimal as possible.

Binaries for Linux, Mac, and Windows

  • We can either build the binaries ourselves and push them to PyPI or test whether Conda can handle this (though I have no experience with Conda). I saw nanobind conda recipes, there should be a way.

GitHub Actions

  • They probably will need to be updated according to current changes.

Contribution Guide

  • Contribution guide has to be updated too.

Why Was compas.h/cpp Renamed to stdafx.h/cpp?

  • I will rename stdafx.h back to compas.h. But it has to be one header file, for this reason all the methods use inline keyword.
  • compas.h will serve as the main header and should ideally be compiled once as a precompiled header:
    Creating precompiled header files
  • scikit-build, that is used by nanobind, helps to connect CMake C compilation with pip installation of Python. I am trying to figure out how to set precompiled headers to speed up the building process for development.

Manual Profiling in Tests

I find the "manual profiling" in the tests a bit messy. Perhaps the tests should focus solely on testing, while separate, dedicated scripts handle profiling (ideally using a profiler).

  • I will clean up the test files.

@petrasvestartas
Copy link
Collaborator Author

petrasvestartas commented Mar 4, 2025

Precompiled headers are working!

After setting this locally:

   pip install nanobind scikit-build-core[pyproject]
   conda install cmake
   pip install --no-build-isolation -ve .
   pip install --no-build-isolation -ve . -Ceditable.rebuild=true

Build time without precompiled headers:
image

Build time with precompiled headers:
image

For precompiled headers, it is always good to place as many 3rd party headers to one single file as possible so that C/C++ code changes in other files would not take too much time to wait.

This example demonstrates how to perform Poisson surface reconstruction from a point cloud with normals using COMPAS CGAL.

Key Features:
* Loading point clouds with normals from XYZ files
Copy link
Member

Choose a reason for hiding this comment

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

i think you need a blank line before the start of a list

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated, all examples were missing this.

@tomvanmele
Copy link
Member

please put the workflows back so we can start running tests on push again

@petrasvestartas
Copy link
Collaborator Author

Ok, let's see what is happening on ubuntu.

@tomvanmele
Copy link
Member

please remove the "if name is main" guards from the examples. they serve no purpose...

Copy link
Member

@tomvanmele tomvanmele left a comment

Choose a reason for hiding this comment

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

Cool!

@tomvanmele tomvanmele merged commit 2ae5bf3 into compas-dev:main Mar 14, 2025
11 checks passed
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