aiidateam / aiida-common-workflows

A repository for the implementation of common workflow interfaces across materials-science codes and plugins
https://aiida-common-workflows.readthedocs.io
MIT License
52 stars 30 forks source link

Update VASP part to comply with ACWF study #303

Closed espenfl closed 1 year ago

espenfl commented 1 year ago

We add dedicated handler overrides and a dedicated protocol that was used for this study. Also, a dedicated potential mapping has been created as the VASP developers was in the process of updating a few potentials, in particular for the lanthanides. Furthermore, we disable the present default to hard stop on CNORMN warnings for this study. This goes in hand with the dedicated v2.2 release of the AiiDA-VASP plugin, which is compatible with aiida_core<2.

espenfl commented 1 year ago

This is marked WIP due to the fact that a release of the new potentials is pending with the VASP developers. Maybe we can update this PR when the release is made to make it easier to reproduce results.

MichaelWolloch commented 1 year ago

The good thing about updating the potentials (only 6 elements would change, so the box plots etc. would be nearly indistinguishable) would be that then the potential family would be much easier to identify as 64 and the potential mapping would be clear, even without looking at the hashes. Now we have 54*, with the asterisk needed to indicate the new lanthanides which will be part of the official 64 potential set.

zhubonan commented 1 year ago

Nice to see this getting merged๐Ÿš€๐Ÿš€

Maybe a bit off topic - in my research I used the potential mapping used by Materials Project a lot. Have you guys done any testing of that?

espenfl commented 1 year ago

Maybe a bit off topic - in my research I used the potential mapping used by Materials Project a lot. Have you guys done any testing of that?

No, we have not done any testing on this.

Also, stating here for future reference that some of the potentials used in this study is for comparisons with all-electron only. For calculations where some kind of property is needed, the state of some of the elements would in almost need to be different to correct for localization issues. Please check the notes about the _2 and _3 potentials at the end of this page: https://www.vasp.at/wiki/index.php/Available_PAW_potentials for more details and guidelines.

espenfl commented 1 year ago

Releasing the WIP.

espenfl commented 1 year ago

@bosonie Seems the handler_override is not accepted here as an input port. It works in AiiDA otherwise I think and we need this for this particular study.

espenfl commented 1 year ago

@bosonie Seems the handler_override is not accepted here as an input port. It works in AiiDA otherwise I think and we need this for this particular study.

Never mind this. Seems okey now.

espenfl commented 1 year ago

Thanks @mbercx. Agree on the suggestions and/or comments. But before making changes I would like to make it clear that I would like us to consider making a release that is dedicated towards the ACWF study. And when doing that I would at present stage like to make as little code changes as possible. We could of course implement those in develop as soon as the release is made.

sphuber commented 1 year ago

@mbercx I agree with @espenfl here. We have a short deadline to get the changes for the verification study in and releases, so let's leave any comments that are not critical to a later stage. Anyway when we have made a release, we will migrate the code to be compatible with AiiDA v2.0 and we can then also go over the existing code and make things consistent for example with respect to unit conversions, coding style etc.

mbercx commented 1 year ago

@espenfl @sphuber Fully on board with this as well! We can come back to my comments at a later time. ๐Ÿ‘

Once the small merge conflict is resolved, feel free to go ahead and merge.

espenfl commented 1 year ago

@mbercx Okey, I will fix this today. Not sure about the aiida-ase. Should that be included or not? We do not need it aiida-vasp side.

espenfl commented 1 year ago

@mbercx @sphuber Will pin it to ~2.2 as we are on ~3.0 for the v2 stuff. Will fix the rest later today.

sphuber commented 1 year ago

@espenfl aiida-ase is necessary for the GPAW implementation that is now also part of the ACWF, so we should keep that

espenfl commented 1 year ago

@mbercx @sphuber All done now. Ready to go. Happy to update all the other stuff after the release.