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

Revamp CMake configuration to "modern" CMake #2113

Closed
rcurtin opened this issue Dec 10, 2019 · 11 comments
Closed

Revamp CMake configuration to "modern" CMake #2113

rcurtin opened this issue Dec 10, 2019 · 11 comments

Comments

@rcurtin
Copy link
Member

rcurtin commented Dec 10, 2019

This issue came about as a result of #2107, which disables Boost's CMake configuration scripts because mlpack's CMake configuration is written in "old-style" CMake.

An introduction to the difference between "old" and "modern" CMake can be found at these resources:

The goal of this issue is very wide-ranging: essentially it means completely overhauling mlpack's CMake configuration so that it is done in the style of modern CMake, and will be compatible with future changes to CMake. Specifically, we should be able to remove the hack of #2107 too.

I don't have a set workflow for solving this issue; it will have to be approached by spending a lot of time with the current CMake configuration and understanding its ins and outs. I won't claim that the current CMake configuration is clean either. A definite gotcha is the bindings configurations, in src/mlpack/bindings/*/CMakeLists.txt. These are tricky CMake scripts that do things in strange ways to get the automatic bindings building, and it may be very difficult to port these safely---especially the Python bindings, which have to do a lot of configuration of dependent files and then call out to a different build system (setuptools).

This may be appropriate as a GSoC project, too. 👍

@knakul853
Copy link
Contributor

@rcurtin book is really nice, I found it very broad but something new and challenging

@abernauer
Copy link

@rcurtin If you want help establishing a proper workflow for handling the issue open to helping. It's a pretty open ended issue and agree the scope is fitting of a GSoC project. I can write up some documentation on best practices for CMake to help whoever eventually tackles the issue.

@knakul853
Copy link
Contributor

@abernauer that would be great to create a workflow, it will help a lot to start with : /

@birm
Copy link
Member

birm commented Dec 20, 2019

I've added an item to the GSOC ideas list just in case no one gets to this first.

@kumaran-14
Copy link

@birm I'd like to work on this as a GSoC project. I have good experience in cmake build system. As a starter, I would like implement a basic c++ project which uses new cmake build features.

@birm
Copy link
Member

birm commented Jan 16, 2020

@kumaran-14 That sounds like a great start! If you haven't already done so, consider joining the mailing list and/or irc: https://www.mlpack.org/community.html

@mlpack-bot
Copy link

mlpack-bot bot commented Feb 15, 2020

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions! 👍

@shubham1206agra
Copy link
Contributor

Is this issue still open or is it done already?

@Aakash-kaushik
Copy link
Contributor

Hey @shubham1206agra I think this issue has been handled but @rcurtin might be able to provide more info on it.

@rcurtin
Copy link
Member Author

rcurtin commented Jan 3, 2022

Sorry for the slow response. This actually is not yet done, but, note that initiatives like #2646 and #2440 mean that the dependencies of mlpack will change significantly in the near future, and thus the CMake configuration will change somewhat too.

I think, if you are interested in working on this, we should find a way to split up the work into small pieces, since mlpack's CMake configuration is quite complicated due to all the bindings. Maybe starting with the base library/headers (and note that libmlpack.so will be gone soon, since we are making mlpack header-only; see #3091), and making it satisfy each of these things is a good start:

https://gist.github.com/mbinna/c61dbb39bca0e4fb7d1f73b0d66a4fd1

Anyway, I hope that's helpful---I suspect this effort will get to be kind of in-depth. :)

@shrit
Copy link
Member

shrit commented Jan 3, 2022

Also, an idea to add on what @rcurtin mentioned, when mlpack becomes header only, we can inspire from the ensmallen cmake configurations since they are (I think) using modern cmake.

@mlpack mlpack deleted a comment from t-dasun May 30, 2023
@mlpack-bot mlpack-bot bot closed this as completed Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants