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

CMake and added functionality #51

Closed
wants to merge 13 commits into from
Closed

CMake and added functionality #51

wants to merge 13 commits into from

Conversation

hochshi
Copy link

@hochshi hochshi commented Feb 22, 2023

Hi,
I've used fgt to create pyfgt a simple pybind11 python wrapper around fgt.

Unfortunately, the local FindEigen3.cmake is unable to find Eigen3 when used when through pip or conda build systems. So, I've added an option (WITH_EIGEN_NOMODULE) that allows users to use find_package(Eigen3 3.1 REQUIRED NO_MODULE) and link against Eigen3 instead of find_package(Eigen3 3.1 REQUIRED). I haven't messed about with FindEigen3.cmake since I'm unaware of the reasons for it's creation and proper usage.

The second addition has to do with easier usage in Coherent Point Drift, where having the entire matrix instead of weighted sums is considerably faster and easier.

Copy link
Owner

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I'd like to know more about why pip/conda can't use the current FindEigen3.cmake -- if possible, I'd prefer to fix the find script, rather than add a new configuration option. Can you post information here or in a new issue re: the find script breakages?

Also, for the new functionality, can you add one or more tests demonstrating correct functionality? Thanks!

@hobu
Copy link
Contributor

hobu commented Apr 12, 2024

Thanks for the contribution! I'd like to know more about why pip/conda can't use the current FindEigen3.cmake -- if possible, I'd prefer to fix the find script, rather than add a new configuration option. Can you post information here or in a new issue re: the find script breakages?

I think the better thing to do is to use CMake's own config machinery like I have implemented in #54.

@gadomski
Copy link
Owner

Closing as OBE by #54

@gadomski gadomski closed this Apr 16, 2024
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