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

Allow using vesin in ASE calculator #659

Merged
merged 4 commits into from
Jun 21, 2024
Merged

Allow using vesin in ASE calculator #659

merged 4 commits into from
Jun 21, 2024

Conversation

frostedoyster
Copy link
Contributor

@frostedoyster frostedoyster commented Jun 18, 2024

The ASE calculator is annoying for very simple potentials (I'm playing with a Morse potential) because the NL becomes slow for dense systems.

EDIT: the speed-up is crazy

Contributor (creator of pull-request) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

Reviewer checklist

  • CHANGELOG updated with public API or any other important changes?

📚 Download documentation preview for this pull-request

@frostedoyster frostedoyster changed the base branch from ase-store to master June 21, 2024 07:27
@frostedoyster frostedoyster force-pushed the vesin-for-ase branch 3 times, most recently from 374e364 to 47bf462 Compare June 21, 2024 07:50
Copy link
Contributor

@Luthaf Luthaf left a comment

Choose a reason for hiding this comment

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

The code looks good, but this ability should be documented somewhere. Maybe in the docstring for the ASE calculator, and maybe in the ASE MD example as well?

@frostedoyster
Copy link
Contributor Author

I added it to the docstring, but I didn't find a good place to add it to the ASE tutorial, simply because it doesn't use a NL :)

@Luthaf
Copy link
Contributor

Luthaf commented Jun 21, 2024

Good point for the tutorial 😄

@Luthaf Luthaf merged commit 234a54c into master Jun 21, 2024
23 checks passed
@Luthaf Luthaf deleted the vesin-for-ase branch June 21, 2024 14:13
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.

None yet

2 participants