ZimmermanGroup / pyGSM

Thermal and photochemical reaction path optimization and discovery
MIT License
50 stars 26 forks source link

coordinate systems and relation to geomeTRIC #30

Open stenczelt opened 3 years ago

stenczelt commented 3 years ago

I understood that some of the coordinate system code has been forked from geomeTRIC, and refactored. I quite like that it lives in multiple files here rather than one long file.

I was going to have a go at trying out and implementing some ideas with coordinate systems, which if work well I would like to commit here and in geomeTRIC as well. Is anyone maintaining changes across these two, how diverged are the two versions of the coordinate system codes? So what do you think is the correct way to go about this? My plan is to develop on the fork of this repo and then port the changes to geomeTRIC as well, how much of a pain will that be?

craldaz commented 3 years ago

It would probably be a challenge.

The main difference is that I'm doing block matrix diagonalization to handle bigger systems and have needed to take extra care to reduce the computational overhead of certain functions; for example, the formation of the primitive internal coordinates.

I would have liked to put this into geomeTRIC but I haven't had the time to do it.

stenczelt commented 3 years ago

I see, since then I took a look and saw that some changes have been made. I am happy to help out with some of this. I was going to add some more periodic boundary support as well, and non-orthogonal cells through ASE's cell and neighbour list definitions.

What I have not found readily in the code is the support for surface reactions, specifying slab as cartesians and the rest with TRIC for example. Can I use -hybrid_indices to specify that?

ms860309 commented 3 years ago

I think supporting periodic boundary can extend the ability of the pyGSM to explore the catalytic reactions. In my impression, the slab reaction is usually through the NEB simulation of VASP and other software. One of the reasons seems to be that DFT csoftware cannot describe slab well? Can the gradient of slab be calculated only with the current level of theories supported by pygsm?

craldaz commented 3 years ago

You can describe a slab with -hybrid_indices. See Jafari, M. & Zimmerman, P. M. Reliable and efficient reaction path and transition state finding for surface reactions with the growing string method. J. Comput. Chem. 645–658 (2017) doi:10.1002/jcc.24720.

The -hybrid_indices are a bit of a misnomer they correspond to the atoms to treat with Cartesian coordinates.

I've never done any ASE slab calculations, can you could upload one to the /examples/ folder?

@ms860309 the DLC/TRIC coordinate system used here is not periodic but the electronic structure theory can be periodic (e.g. OpenMM, ASE, etc). I don't think this should be a major problem unless the lattice vectors are changing a lot. This publication Bučko, T. Transition state optimization of periodic systems using delocalized internal coordinates. Theor. Chem. Acc. 137, 1–10 (2018 describes how one can make DLCs/TRIC periodic by introducing lattice parameters. However, it seems too complicated for me to implement easily.

stenczelt commented 1 year ago

@craldaz getting back to this, here is an example of with ASE's NEB:

https://github.com/stenczelt/pyGSM/commit/d2e0f30d70960888db377cb9cfb4e8fdcf244cc2

I have noticed that using the hybrid indices, the code breaks at the point of adding the end nodes:

Traceback (most recent call last):
  File "/Users/tks32/opt/miniconda3/envs/geometric-xtb/bin/gsm", line 33, in <module>
    sys.exit(load_entry_point('pyGSM', 'console_scripts', 'gsm')())
  File "/Users/tks32/work/gsm-repos/pyGSM/pygsm/wrappers/main.py", line 712, in main
    gsm.go_gsm(inpfileq['max_gsm_iters'], inpfileq['max_opt_steps'], rtype)
  File "/Users/tks32/work/gsm-repos/pyGSM/pygsm/growing_string_methods/de_gsm.py", line 40, in go_gsm
    self.add_GSM_nodes(2)
  File "/Users/tks32/work/gsm-repos/pyGSM/pygsm/growing_string_methods/de_gsm.py", line 94, in add_GSM_nodes
    self.add_GSM_nodeR()
  File "/Users/tks32/work/gsm-repos/pyGSM/pygsm/growing_string_methods/main_gsm.py", line 539, in add_GSM_nodeR
    self.nodes[self.nR] = GSM.add_node(
  File "/Users/tks32/work/gsm-repos/pyGSM/pygsm/growing_string_methods/gsm.py", line 445, in add_node
    new_xyz = nodeR.coord_obj.newCartesian(old_xyz, dq0)
  File "/Users/tks32/work/gsm-repos/pyGSM/pygsm/coordinate_systems/internal_coordinates.py", line 412, in newCartesian
    xyz2 = xyz1 + dxyz.reshape((-1, 3))
ValueError: operands could not be broadcast together with shapes (13,3) (17,3) 
stenczelt commented 1 year ago

I came across https://github.com/mina-jafari/surfaceGSM - have all the functionalities from here been integrated into the Python code?