Reduce species copying and resonance structure generation when reacting #1677
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1677 +/- ##
==========================================
+ Coverage 41.78% 41.79% +0.01%
==========================================
Files 177 177
Lines 30207 30206 -1
Branches 6252 6252
==========================================
+ Hits 12622 12625 +3
+ Misses 16661 16658 -3
+ Partials 924 923 -1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1677 +/- ##
==========================================
+ Coverage 41.78% 41.79% +0.01%
==========================================
Files 177 177
Lines 30207 30206 -1
Branches 6252 6252
==========================================
+ Hits 12622 12625 +3
+ Misses 16661 16658 -3
+ Partials 924 923 -1
Continue to review full report at Codecov.
|
rmgpy/data/kinetics/kineticsTest.py
Outdated
| @@ -1091,3 +1091,97 @@ def test_add_atom_labels_for_reaction_3(self): | |||
| self.assertIn('*2',found_labels) | |||
| self.assertIn('*3',found_labels) | |||
|
|
|||
| def test_species_preserved_after_generate_reactions(self): | |||
| """Test that Species objects do not retain changes after generating reactions""" | |||
| r1 = Species(index=1, label='ethyl', SMILES='C[CH2]') | |||
There was a problem hiding this comment.
Can you use something more descriptive than r1? We more commonly use r for reaction than reactant, which can result in some confusion here.
There was a problem hiding this comment.
I feel like it should be clear from context, but I changed them anyway.
rmgpy/data/kinetics/kineticsTest.py
Outdated
| self.assertIsNot(r1, r2_out) | ||
|
|
||
| # The molecule objects should be the same references for the first output species | ||
| self.assertIsNot(r1.molecule[0], r1_out.molecule[0]) |
There was a problem hiding this comment.
Why is one of them the molecule objects the same reference and the other isn't? Why can't all identical molecule objects be the same reference?
There was a problem hiding this comment.
If they were the same object then you couldn't assign different labels to the atoms.
Test that Species objects passed to generate_reactions_from_families do not retain any modifications after reaction generation
Motivation or Problem
Two performance related changes, cherry-picked from #1577.
Description of Changes
react_species(reasoning)Testing
Tested with the liquid oxidation test from RMG-tests, but on the FilterCritPerReactFamily branch.
Should confirm that RMG-tests show no differences in models except for runtime.