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

sklearn tests #221

Closed
wcwitt opened this issue Aug 12, 2024 · 6 comments
Closed

sklearn tests #221

wcwitt opened this issue Aug 12, 2024 · 6 comments

Comments

@wcwitt
Copy link
Collaborator

wcwitt commented Aug 12, 2024

Hi @cortner, I see you disabled some sklearn tests recently, possibly because they were failing with the new backend?

# @testset "SKLEARN_ARD" begin

Noam (with my help) is trying to reproduce some past fits and we need this functionality ... would it be best to work from an older ACEpotentials?

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

Here's the commit that disabled them: f6d42d3.

@cortner
Copy link
Member

cortner commented Aug 12, 2024

they have been failing for a long time. It has nothing to do with the new the backend, I actually haven't retired the old backend yet.

I disabled them because I wanted to rethink carefully about how to re-integrate SKLEARN parameter estimation. Likely through an extension or through some other generic Julia linear regression packages.

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

That's helpful - maybe it has to do with the MLJ extension. I'll try to figure it out.

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

This issue will be partly addressed by ACEsuit/ACEfit.jl#77.

I disabled them because I wanted to rethink carefully about how to re-integrate SKLEARN parameter estimation. Likely through an extension or through some other generic Julia linear regression packages.

Happy to discuss this, but at the moment sklearn is available only through extensions. Although it's a little confusing because there are two routes: one utilizing PythonCall as an extension and the other through MLJ as an extension.

@cortner
Copy link
Member

cortner commented Sep 9, 2024

If thought about this and I'd like to avoid installing all the python dependencies for the ACEpotentials tests so would prefer not to re-instate those. If you disagree with me please re-open and we can discuss again.

@cortner cortner closed this as completed Sep 9, 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

No branches or pull requests

2 participants