grimme-lab / xtb-python

Python API for the extended tight binding program package
https://xtb-python.readthedocs.io
GNU Lesser General Public License v3.0
102 stars 30 forks source link

Periodic support in ASE calculator #34

Open rlaplaza opened 4 years ago

rlaplaza commented 4 years ago

Hello,

I was trying to use the ASE calculator in PBC with XTB using the GFN0-xTB parameters, which seem to work OK for the systems that I am studying.

The ASE calculator is, as far as I understand, not supporting such info because the cell and pbc info in the Atoms object are not parsed, but should be quite straightforward to pass to the underlying Calculator that connects with the XTB API. Actually, this seems to work if I do it directly with a simple wrapper.

I wondered why (if there is a reason) this has not been done yet. I might do it myself, which I guess would only require a bunch of pure python in the ASE calculator file.

I might also want to have a verbosity flag passed to the ASE calculator so that it is not actually mute, because sometimes I might care about the outputs of the XTB part. Again, does not seem to be a lot of work and I could do it myself by passing a verbosity attribute in the Atoms object. Does that seem reasonable?

In any case, thank you and the group for this mighty, mighty effort.

awvwgk commented 4 years ago

The setup of the xtb API objects is currently done here:

https://github.com/grimme-lab/xtb-python/blob/fdaa42b20fb099a3e56230f85fe8c8dccea1fe5a/xtb/ase/calculator.py#L188-L206

I thought I managed the periodic case as well by passing the cell object, but I may be mistaken.

Also, feel free to share any improvements to the ASE calculator.

rlaplaza commented 3 years ago

I was wrongly assuming stuff.

The passing seems to be proper, but atoms near the PBC borders experience weird forces (as described in https://github.com/grimme-lab/xtb/issues/257 ), which sometimes lead to immediate segfaults on the xtb side (think about explicit solvent more than solid state stuff, which is more well-behaved). In this sense, I think the possibility of passing other verbosity settings to the ASE calculator is desirable (e.g. this), as it was very revealing in this case.

I think the GFN0-xTB hamiltonian has some symmetry issues. The activation energy of my rusty Fortran coding skills is too high for the time being, but I am pretty sure that simulations with xtb 6.2 did not have this issue, so the key diff might be in between. I'll test more thoroughly.

On that note, I wonder whether you guys feel like this is a priority for the team at the moment or the whole PBC issue is "not urgent".

awvwgk commented 3 years ago

On that note, I wonder whether you guys feel like this is a priority for the team at the moment or the whole PBC issue is "not urgent".

Sorry, “the team” is mostly just me and I currently can't say when I'm able to allocate the necessary time to focus on the PBC implementation again.

rlaplaza commented 3 years ago

Just a quick update: it's definitely still breaking in 6.2.0, therefore its a more intricate thing for sure. Whenever you tackle it, let me know and I'll try to help with testing and examples.

Sorry, “the team” is mostly just me and I currently can't say when I'm able to allocate the necessary time to focus on the PBC implementation again.

You are doing a great job! Courage!

rlaplaza commented 3 years ago

Some tests for the record, all with the GFN0 hamiltonian:

xtb-6.3.3_tests.tar.gz

xtb-6.2.0_tests.tar.gz

Thus, i'd say that some of the updates between 6.2.0 and 6.3.3 messed up something in the dispersion term for this hamiltonian at least. As there was a bugfix before, i'd say that the bugfix has a bug itself. The optimizer issue, tho, I have no idea.