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

Question: Version the C++ implementation under a dedicated repository #1

Open
jjerphan opened this issue Dec 9, 2023 · 5 comments
Open

Comments

@jjerphan
Copy link

jjerphan commented Dec 9, 2023

Dear @yixuan and @statmlben,

Thank you for this truly magnificent piece of work.

Currently, both https://github.com/softmin/ReHLine-r and https://github.com/softmin/ReHLine-python vendor the C++ implementation.

I think having a third repository (for instance https://github.com/softmin/ReHLine-cpp) version the C++ implementation separately would allow it to:

What do you think of this proposal?

Best Regards,
Julien.

@yixuan
Copy link
Contributor

yixuan commented Dec 10, 2023

Hi Julien, you can check out our ReHLine-SVM repository, which is a basically a single-header C++ library. Although it is tagged as an SVM solver, its core part is the function rehline::rehline_solver(), and the SVM solver is a simple wrapper of it. I'll try to add more documentation on the C++ interface, but most of the arguments should be straightforward.

@statmlben
Copy link
Member

Thank you for your encouragement toward ReHLine! As you pointed out, we share the same rehline.h for both R and Python. Yixuan and I had a quick discussion on incorporating your comments: we are considering to update/expand the current ReHLine-SVM, containing only a single C++ header, into a version as ReHLine-cpp.

@jjerphan
Copy link
Author

Thank you for your prompt answers!

What do you think of having ReHLine be distributed on conda-forge?

It might be possible to distribute it under several packages, for instance:

  • rehline-cpp (or librehline or just even rehline) for the core C++ implementation
  • rehline-python for the Python bindings which would depend on rehline-cpp
  • rehline-r for the R bindings which would also depend on rehline-cpp

If you find it valuable, I can help when I get some time.

@yixuan
Copy link
Contributor

yixuan commented Dec 13, 2023

Yeah, thanks for the kind help. We are still considering how to refactor the code and repository to make them more structured. Some current considerations are as follows:

  1. The core of rehline-cpp may be just one header file. Not sure if it is an overkill to distribute rehline.h as a separate package.
  2. Python and R packages have their own building system, so I feel that rehline-python and rehline-r do not need to depend on rehline-cpp. It would be easier to just include rehline.h in the Python/R source package.
  3. The Python and R packages are still in their early stages, and APIs may change a lot. It might be more responsible to first stabilize the interface before formally submitting to Conda/PyPI/CRAN.

If you have any thoughts or suggestions, feel free to leave comments here, and we really appreciate that.

@jjerphan
Copy link
Author

  1. The core of rehline-cpp may be just one header file. Not sure if it is an overkill to distribute rehline.h as a separate package.

This would allow people to use it via package manager's distribution.

  1. Python and R packages have their own building system, so I feel that rehline-python and rehline-r do not need to depend on rehline-cpp. It would be easier to just include rehline.h in the Python/R source package.

They indeed do not need to indeed. It might help having the implementation evolve independently from the interface, but this is slightly more involved from a packaging and maintenance point of view.

  1. The Python and R packages are still in their early stages, and APIs may change a lot. It might be more responsible to first stabilize the interface before formally submitting to Conda/PyPI/CRAN.

I agree.

I let you close this issue if you think it is resolved.

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

No branches or pull requests

3 participants