search

libAtoms / workflow

python workflow toolkit
GNU General Public License v2.0
32 stars 18 forks source link

The Vasp calculator with pbc = False runs VASP multiple times #258

Closed bernstei closed 10 months ago

bernstei commented 1 year ago

The wfl Vasp calculator wrapper changes all the pbcs to True before actually running VASP since the ASE Vasp calculator complains otherwise, and then changes back once the calculation is done. However, the saving of the properties triggers additional VASP runs, basically because the ASE system detects the change in pbc values as a change to the system and invalidates the cache when save_results() calls things like get_potential_energy().

For VASP it'd be easy enough to make save_results() try to grab the values from atoms.calc.results, which won't trigger another evaluation. I'm not sure how that'd behave with CASTEP, which doesn't have a Calculator.results dict.

bernstei commented 1 year ago

Although, I'm a bit confused, because it should fail when it tries to rerun with the non-True pbc.

[edited] Apparently something entirely inexplicable is happening. In the calculates that are automatically triggered the atoms object ends up with pbc True, despite the fact that the one that's getting passed in has pbc False. It just changes value for no apparent reason. I can print atoms.pbc in BaseCalculators.get_property and it's [False] * 3, but as far as I can tell it calls Vasp.calculate, and it gets atoms.pbc of [True] * 3, despite the fact that as far as I can tell there's nothing in between except entering the function.

[edited more] I can't do a simple reproduction. I wonder if it has something to do with the fact that these modifications of pbc are happening inside of the derived class Vasp.calculate, and so the changes are happening in weird places relative to where the testing for changed atomic configurations is done.

bernstei commented 1 year ago

I've worked around this, for now at least, by using Calculator.results in save_results() if available. Works for VASP, won't work for CASTEP, but the issue is, so far as I can tell, only happening if the pbc manipulation is being done. That manipulation is required for ase.calculators.vasp.Vasp, so our wrapper does it, while the ASE Castep apparently just overwrites it or something, so that'll have to be handled differently.

bernstei commented 1 year ago

I think I found a cleaner solution. PR coming soon.