Test that we can make a sample molecule for every atom type. #1661
Conversation
|
Just updated to RMG 3. |
|
This PR is abandoned but up-to-date and simple enough that it could be merged. I am going to change the base branch to |
JacksonBurns
force-pushed
the
atomtypetest
branch
from
4e78950
to
16b09b0
Compare
|
Rebased and CI is now running. |
|
Couldn't make sample molecules for types ['Ctc', 'N5bd', 'O4b', 'P3b', 'P5b', 'P5bd', 'S4b'] |
|
Now down to ['N5bd', 'O4b', 'P3b', 'P5b', 'P5bd', 'S4b']. |
|
Got several more working. |
Codecov Report
@@ Coverage Diff @@
## main #1661 +/- ##
==========================================
- Coverage 48.11% 48.11% -0.01%
==========================================
Files 110 110
Lines 30626 30625 -1
Branches 7988 7988
==========================================
- Hits 14735 14734 -1
+ Misses 14363 14362 -1
- Partials 1528 1529 +1 see 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
JacksonBurns
force-pushed
the
atomtypetest
branch
from
0b18290
to
dac9888
Compare
This unit test was inspired by efforts to debug something in some
changes to the database, which caused a segfault when running
the database Tests, but was in fact caused by an undetected bug in RMG. This should detect similar bugs.
The idea is that for every atomType, such as 'Cds', the
method Group().make_sample_molecule() can create a molecule
from the group definition, eg.
1 Cd ux
should make the molecule
1 C u0 p0 c0 {2,D} {3,S} {4,S}
2 C u0 p0 c0 {1,D} {5,S} {6,S}
3 H u0 p0 c0 {1,S}
4 H u0 p0 c0 {1,S}
5 H u0 p0 c0 {2,S}
6 H u0 p0 c0 {2,S}
Many work, but quite a few fail.
Updated to RMG3/Python3.
We have a hard-coded list of charged atom types here. It was missing one.
The atomtypes each have a charge list - let's just see if it matches.
When you create a molecule from a functional group definition that
declares implicit benzene rings by using a "benzene" atom type,
it creates benzene rings.
This algorithm contains a list of "benzene" atom types.
It's hard-coded. And missing some. This adds one, to check that
adding one fixes things.
With this fix, the following line works:
rmgpy.molecule.Group().from_adjacency_list('1 N5bd ux').make_sample_molecule()
Before it had a hard-coded list (that was missing some). Now any atomtype that is declared as having at least one benzene bond is counted.
This helps generating sample molecules for certain atomtypes.
Now lines like this work:
rmgpy.molecule.Group().from_adjacency_list('1 P3b ux').make_sample_molecule()
Fixing O4b and S4b will be nontrivial, so rather than fix them I'm marking them as "work in progress" (expected to fail)
Ideally we could make a molecule from every atom type definition that matches that atom type. This is a work in progress test, because we can't. Two (marked in previous tests) crash. Some others create molecules, but they don't seem to match the functional group they were designed to match.
Motivation or Problem
This unit test at the start of this PR was inspired by efforts to debug something in some changes to the database (ReactionMechanismGenerator/RMG-database#317) , which caused a segfault when running the database Tests, but was in fact caused by an undetected bug in RMG (#1656) not a problem in the database. I felt there ought to have been a unit test to prevent the bug.
Description of Changes
The goal is to make a group definition out of each atom type, and then make a sample molecule for each group definition. This should test the group definition and the
make_sample_moleculemethod.At first, the test failed, and that was either because of
make_sample_moleculemethod.I then fixed several of the bugs - mostly in the algorithm to make aromatic rings for sample molecules.
The remaining bugs I split into a "work in progress" unit test - they involve aromatic O and S atoms (['O4b', 'S4b'] ) which seem to be poorly supported currently.
I then made a more stringent test - that not only do we generate sample molecules without crashing, but that they're correct. This test is also currently marked a "work in progress". The list of problems is Xo, Cq, Sib, Sibf, Siq. Fixing those can be a separate issue.