Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install c++ headers and CMake package #40

Closed
wants to merge 1 commit into from

Conversation

ordinary-slim
Copy link
Contributor

@ordinary-slim ordinary-slim commented Dec 2, 2024

I need to interface with multiphenicsx at the c++ level so I modified the CMakeLists.txt following the CMake guide on importing and exporting. It now installs headers (right now only DofMapRestriction.h) and a findable CMake package so that it can be used in other projects. Here is a sample project.

Disclaimer: I've only tried it on the docker container I'm working on.

@ordinary-slim
Copy link
Contributor Author

I got around trying it on another machine and made some changes. As of now you need to run cmake --install (build_dir_path) in order to install the headers and the cmake package. I am not sure how to set it up from the pyproject.toml

@francesco-ballarin
Copy link
Member

Thanks @ordinary-slim . Unfortunately, I am not fully convinced by this, and I will be closing it without merging.

When I look at the installed files I see

./include:
multiphenicsx

./include/multiphenicsx:
DofMapRestriction.h

./lib:
cmake

./lib/cmake:
multiphenicsx

./lib/cmake/multiphenicsx:
MULTIPHENICSXConfig.cmake  MULTIPHENICSXConfigVersion.cmake  multiphenicsxTargets.cmake  multiphenicsxTargets-noconfig.cmake

./multiphenicsx:
cpp

./multiphenicsx/cpp:
multiphenicsx_cpp.cpython-312-x86_64-linux-gnu.so

which is odd for two reasons:

  1. the library multiphenicsx_cpp.cpython-312-x86_64-linux-gnu.so is not a lib but somewhere else. This is probably fixed very easily
  2. the library multiphenicsx_cpp.cpython-312-x86_64-linux-gnu.so contains the nanobind wrappers. If our goal is to make a C++ multiphenicsx package, then
nanobind_add_module(
  multiphenicsx_cpp
  NOMINSIZE
  multiphenicsx/fem/DofMapRestriction.cpp
  multiphenicsx/fem/sparsitybuild.cpp
  multiphenicsx/la/petsc.cpp
  multiphenicsx/wrappers/fem.cpp
  multiphenicsx/wrappers/la.cpp
  multiphenicsx/wrappers/multiphenicsx.cpp
)

currently in CMakeLists.txt would have to be split into

  multiphenicsx/fem/DofMapRestriction.cpp
  multiphenicsx/fem/sparsitybuild.cpp
  multiphenicsx/la/petsc.cpp

going into a proper libmultiphenicsx.so and

  multiphenicsx/wrappers/fem.cpp
  multiphenicsx/wrappers/la.cpp
  multiphenicsx/wrappers/multiphenicsx.cpp

going into multiphenicsx_cpp.cpython-312-x86_64-linux-gnu.so. Then libmultiphenicsx.so would be installed, while multiphenicsx_cpp.cpython-312-x86_64-linux-gnu.so would not.

pyproject.toml would still be the way to install the library for the normal user, while

As of now you need to run cmake --install (build_dir_path) in order to install the headers and the cmake package. I am not sure how to set it up from the pyproject.toml

would be perfectly fine for an advanced user.

If you manage to restructure the code in this way, and are willing to add at least one simple C++ test that I would then be using on CI, I'd be happy to reconsider closing this. Unfortunately I don't have time to put the effort myself into this task, but please stay in touch if you want to keep working on this :) !

@ordinary-slim
Copy link
Contributor Author

Ok got it. If I give it another try I will probably move the wrappers and python files to a python folder, the c++ files to a c++ folder and have 2 separate CMake files.

@francesco-ballarin
Copy link
Member

I understand the logic of doing that but, if possible, do not do that. I don't want my end users to have to build the c++ layer separately from the python one (as dolfinx does).

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