grimme-lab / xtb

Semiempirical Extended Tight-Binding Program Package
https://xtb-docs.readthedocs.io/
GNU Lesser General Public License v3.0
574 stars 144 forks source link

GFN1-xTB ASE calculator does not currently support PBCs #85

Closed Andrew-S-Rosen closed 4 years ago

Andrew-S-Rosen commented 4 years ago

Describe the bug You may already be aware of this since I know the PBC integration is a work in progress, but if not: GFN1-xTB does not currently work with the ASE interface when PBCs are present. A segmentation fault occurs ("364302 Segmentation fault"). GFN1-xTB works correctly when called from the detailed input interface. This is for xtb version 6.3.0 (preview).

To Reproduce

from xtb import GFN1
from ase.io import read

mol = read('test.cif')
calc = GFN1()
mol.set_calculator(calc)
mol.get_potential_energy()

Expected behaviour Calculation of potential energy

Additional context test.cif

awvwgk commented 4 years ago

~Works fine for me.~ Please make sure that you follow the setup instructions for xtb.

Andrew-S-Rosen commented 4 years ago

Everything is set up according to the instructions (including updating the out-of-date $LD_LIBRARY_PATH to have lib64 instead of lib). The ASE interface works fine for GFN0, but I am still getting the same error for GFN1. I have set ulimit -s unlimited and OMP_STACKSIZE=2G. I even tried using a clean Anaconda environment with a new version of ASE, but no luck. The gfn1.out file starts to be written but then aborts. I tried this on two separate machines just as a sanity check.

           ------------------------------------------------- 
          |                  G F N - x T B                  |
          | Geometry, Frequencies, Noncovalent interactions |
          |            JCTC 2017 parametrisation            |
           ------------------------------------------------- 
             KAB for M(3d)-M(3d) :                1.1000
             KAB for M(4d)-M(4d) :                1.2000
             KAB for M(5d)-M(5d) :                1.2000
             k(s)                :                1.8500
             k(p)                :                2.2500
             k(d)                :                2.0000
             k(f)                :                0.0000
             kEN (H0ij)          :                0.7000
             D3 a1               :                0.6300
             D3 a2               :                5.0000
             D3 s6               :                1.0000
             D3 s8               :                2.4000
             D3 s9               :                0.0000
             alphaj              :                2.0000
             XBdamp              :                0.4400
             XBrad               :                1.3000
             kcnsh               :      0.6000   -0.3000   -0.5000    0.5000

  Z AO/shell   Hii/eV     exponent
  6     Sat Nov  4 20:25:57 CET 2017    EN: 2.550 GAM: 0.480  GM3: 0.1054
     2s    -13.587210    1.960324
     2p    -10.052785    1.832096
ghifi37 commented 4 years ago

There was a class named GFN0_PBC for PBC calculation in old version 6.2, maybe you should use it. But GFN0_PBC is gone after version 6.2.1. It seems the developers are trying to debug it, all we can do maybe just waiting.

Andrew-S-Rosen commented 4 years ago

I can actually get GFN0 to work fine with PBC calculations using the GFN0 calculator (without using GFN0_PBC). I just saw that in the version 6.3.0 (pre-release) that PBC support was added for GFN1, but I can't get it to work with ASE yet. Unfortunately, I'm not sure how to narrow down the issue any further with the binaries, so we'll wait and see. Since GFN2 will also have PBCs in a future release, it may get fixed then.

ghifi37 commented 4 years ago

The python files in pre-release 6.3.0 are still 6.2.1, maybe not updated, so can not support GFN1? Hope GFN1_PBC and GFN2_PBC come together!

awvwgk commented 4 years ago

@arosen93 Apparently, the version I shipped was not the version that I wanted in there. So I am very sorry for shipping faulty code.

@ghifi37 The python side is correctly updated, since the ASE calculator is somewhat agnostic to it and the implementation is hidden in the xtb.interface module. The problem is that I did not pick the patch for the C-API into this preview, so there is no way to fix it without recompiling.

I am not going to tackle this before Christmas (today), but I am planning to push the PBC branch on GH soon and finalize the implementation in January both for xtb and dftb+.

ghifi37 commented 4 years ago

@arosen93 Apparently, the version I shipped was not the version that I wanted in there. So I am very sorry for shipping faulty code.

@ghifi37 The python side is correctly updated, since the ASE calculator is somewhat agnostic to it and the implementation is hidden in the xtb.interface module. The problem is that I did not pick the patch for the C-API into this preview, so there is no way to fix it without recompiling.

I am not going to tackle this before Christmas (today), but I am planning to push the PBC branch on GH soon and finalize the implementation in January both for xtb and dftb+.

Thank you for your hardworking and Merry Chrismas!

awvwgk commented 4 years ago

I updated the preview with a new binary, the GFN1-xTB problems is now fixed there.