ACEsuit / ACEfit.jl

Generic Codes for Fitting ACE models
MIT License
7 stars 6 forks source link

PyCall #51

Closed cortner closed 1 year ago

cortner commented 1 year ago
cortner commented 1 year ago

CC @tjjarvinen -- I believe this would take you no time at all, and fits with your general work on the python interface.

tjjarvinen commented 1 year ago

Ok, so the idea is that you want Python SkLearn to be an option for fitting, but normally you would use Julia Optim.jl?

For what I looked you did not use ScikitLearn.jl, but instead called SkLearn from Python with PyCall. This will make it easy to convert to PythonCall.

cortner commented 1 year ago

correct. At the moment, some people still use this, but over time there will likely be fewer and fewer as Chuck's code is improving.

wcwitt commented 1 year ago

I’m fine with this, but note that PyCall is a now a Requires dependency, in case you have to undo that.

On Mon, 19 Jun 2023 at 18:32, Christoph Ortner @.***> wrote:

correct. At the moment, some people still use this, but over time there will likely be fewer and fewer as Chuck's code is improving.

— Reply to this email directly, view it on GitHub https://github.com/ACEsuit/ACEfit.jl/issues/51#issuecomment-1597466564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACXHHTS3H7FFWZ7CNR6CMELXMB5IHANCNFSM6AAAAAAZMFBJVU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

cortner commented 1 year ago

Thanks. I'll check whether PyCall can be removed from [dep] before merging and tagging

cortner commented 1 year ago

I was unable to remove the PyCall and don't have time to figure this out, so I removed Requires instead, sorry about that.

tjjarvinen commented 1 year ago

In my opinnion we can close this issue, as #53 has implemented this.

wcwitt commented 1 year ago

Makes sense - closing