search

MobleyLab / chemper

Repository for Chemical Perception Sampling Tools
MIT License
19 stars 10 forks source link

Add handling for symmetry around force field parameters #50

Closed bannanc closed 5 years ago

bannanc commented 5 years ago

The changes in this PR address issue #36.

Currently this is done by basically brute force:

  1. add first set of atoms to make a graph
  2. use ChemicalEnvironments to check what type of SMIRKS it is
  3. assign the symmetry function based on the environment type
  4. use the scoring function that was written for adding layers to chose the best option using the criteria described by type below.
    • all of these functions use find_pairs that scores a set of atoms and bonds based on potential storage objects for those atoms and bonds to be paired with.

No symmetry necessary

If the environment type is "Atom" or None then the smirks atoms are assigned to storage based on the input. This handles all custom cases and single atom SMIRKS which have no symmetry operation.

Bond

The bond_symmetry function checks for how the two atoms match with existing storage. since our current scoring uses atom and bond pairs the same central bond is used in each. Essentially this means the pairing is done for just the atoms.

Angles and Improper Torsions

The atom_symmetry function checks for how the outer two atoms match with existing storage. This includes the bond between the outer atom and the center atom. The improper_torsion_symmetry does the same thing, but considering all three outer atoms.

Proper Torsions

The proper_torsion_symmetry users the inner atoms and their external bonds. So if you have a torsion (0,1,2,3) it considers two pairs:

  1. atom 1 with bond (0,1)
  2. atom 2 with bond (2,3) The central bond will be central no matter what. The assumption here is that the inner atoms are more important to determining the symmetry than the outer atom. However, in an ideal world you would consider bond atoms and the outer bond when clustering, the problem with that is that it would require writing a completely new pairing function. Or at the very least significantly reworking the current one. For now, I am going to assume the inner atoms are sufficient and write an issue for considering a new pairing function for torsions in the future.
codecov[bot] commented 5 years ago

Codecov Report

Merging #50 into master will decrease coverage by 2.36%. The diff coverage is 44.18%.

bannanc commented 5 years ago

I am worried about the number of open PRs that build on each other. I am going to merge them without review. Any suggested changes from reviews will be added in the next round of PRs.