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

Add support for per-atomic-config calculator in generic.calculator. #254

Merged
merged 23 commits into from
Sep 22, 2023

Conversation

bernstei
Copy link
Contributor

Enables autoparallelization over calculator parameters by optionally constructing per-atom calculator in autoparallelized op wfl.calculators.generic.calculate(), using Atoms.info WFL_CALCULATOR_INITIALIZER, _ARGS, and _KWARGS.

Closes #253

Enables autoparallelization over calculator parameters by optionally
constructing per-atom calculator in autoparallelized op
wfl.calculators.generic.calculate(), using Atoms.info
WFL_CALCULATOR_INITIALIZER, _ARGS, and _KWARGS.
@bernstei bernstei marked this pull request as draft July 26, 2023 14:24
@bernstei
Copy link
Contributor Author

@Felixrccs if you are interested in using this, could I convince you to put together a pytest (could, but doesn't have to be based on a DFT calculator)?

@Felixrccs
Copy link
Contributor

Yes, I can try to make a pytest. (I have no experience with that yet.)

@bernstei bernstei marked this pull request as ready for review August 18, 2023 18:15
@bernstei
Copy link
Contributor Author

Yes, I can try to make a pytest. (I have no experience with that yet.)

If you want a hand, just ask. Relevant example tests are in tests/calculators/

@Felixrccs
Copy link
Contributor

If you want a hand, just ask.

My pytest is in #261.

@bernstei
Copy link
Contributor Author

bernstei commented Sep 6, 2023

@Felixrccs there's an issue with the vasp test. Do you need me to debug it, or have you figured out how to run the vasp tests locally on your machine to fix it?

@bernstei
Copy link
Contributor Author

@Felixrccs any progress cleaning up the vasp test?

@Felixrccs
Copy link
Contributor

Sry I had some busy weeks, I try to do it tomorrow.

@bernstei
Copy link
Contributor Author

Thanks - just wanted to make sure that it wasn't forgotten

@Felixrccs
Copy link
Contributor

Ok I still have calculations running and don't want to change my code meanwhile, I hope I get it done on the weekend.

@Felixrccs
Copy link
Contributor

Felixrccs commented Sep 22, 2023

I fixed it in #265. This should work now, thank for your patience.

@bernstei
Copy link
Contributor Author

@Felixrccs thanks for all the work. I'll merge this one once the CI tests pass. Then, hopefully, we can just update and merge the universal k-point spacing functionality.

@bernstei bernstei merged commit e03f3f2 into main Sep 22, 2023
1 check passed
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.

autoparallelization over different calculation setting
4 participants