Enable Restarting RMG Jobs from a Seed Mechanism by amarkpayne · Pull Request #1641 · ReactionMechanismGenerator/RMG-Py

amarkpayne · 2019-06-24T20:33:37Z

Motivation or Problem

There are many instances where it is desirable to restart an RMG job from the last iteration instead of having to start the mechanism generation from the beginning. RMG currently has a restart file, but it has not been maintained and will be deprecated starting in the next release. Furthermore, these restart files are extremely slow.

The agreed upon solution to this problem is to restart RMG jobs from the seed mechanism files that RMG already outputs. These files allow for the complete reconstruction of the model core and edge at the time of the restart. The only missing piece of information missing is the filter tensors, which help RMG decide which species to react, which species have already reacted, and which species not to react because their expected rate is well below what would be included based on the tolerance. This PR saves these filter tensors and allows for them to be loaded so that RMG jobs can be restarted using this technique

Description of Changes

Reorganized when seed mechanisms are saved
Save the filter tensors (and necessary mappings) to a Filters sub folder of the seed mechanism
Add in a block to the RMG input file format for telling RMG to perform a restart job from a seed mechanism
Automatically generate input files entitled restart_from_seed.py that can be run to restart the job
Updated the documentation to describe how the user can use this restart feature
Saving the seed mechanism each iteration is now the default behavior

Testing

I have restarted the superminimal and minimal examples using this technique, and the same mechanism was obtained if the job was run to completion or if the job was restarted from a seed mechanism from an intermediate iteration seed mechanism.

I have not added anything in the way of unit testing yet, but I am happy to add tests for the crucial features of this PR.

codecov · 2019-06-25T02:35:50Z

Codecov Report

Merging #1641 into master will decrease coverage by 0.13%.
The diff coverage is 17.82%.

@@            Coverage Diff             @@
##           master    #1641      +/-   ##
==========================================
- Coverage   41.79%   41.66%   -0.14%     
==========================================
  Files         177      176       -1     
  Lines       30207    30215       +8     
  Branches     6252     6256       +4     
==========================================
- Hits        12625    12588      -37     
- Misses      16659    16703      +44     
- Partials      923      924       +1

Impacted Files	Coverage Δ
rmgpy/solver/files/listener/input.py	`100% <ø> (ø)`	⬆️
rmgpy/solver/files/liquid_phase_constSPC/input.py	`100% <ø> (ø)`	⬆️
rmgpy/tools/data/sim/simple/input.py	`100% <ø> (ø)`	⬆️
rmgpy/tools/data/sim/mbSampled/input.py	`100% <ø> (ø)`	⬆️
rmgpy/tools/data/sim/liquid/input.py	`100% <ø> (ø)`	⬆️
rmgpy/rmg/model.py	`41.07% <0%> (-0.63%)`	⬇️
rmgpy/stats.py	`59.81% <0%> (+0.33%)`	⬆️
rmgpy/rmg/main.py	`21.85% <15.78%> (-1.23%)`	⬇️
rmgpy/rmg/input.py	`36.58% <7.14%> (-1.36%)`	⬇️
rmgpy/data/kinetics/library.py	`43.13% <78.57%> (+0.03%)`	⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04d4d42...3a00ee3. Read the comment docs.

rmgpy/rmg/input.py

rmgpy/rmg/main.py

examples/rmg/commented/input.py

amarkpayne

Thanks for the helpful comments @mliu49 ! There are a couple of things I need to change depending on what we want to do with the old style restart

rmgpy/rmg/main.py

examples/rmg/commented/input.py

mjohnson541

In general I think this looks like it avoids a lot of the bugs that could easily occur with this feature. I've brought up a few issues I noticed below.

rmgpy/rmg/main.py

documentation/source/users/rmg/input.rst

mliu49

Are the changes to seed paths done as well?

documentation/source/users/rmg/input.rst

rmgpy/rmg/input.py

rmgpy/rmg/main.py

documentation/source/users/rmg/input.rst

rmgpy/rmg/main.py

examples/rmg/commented/input.py

rmgpy/rmg/input.py

mjohnson541

Couple small additional things.

rmgpy/rmg/main.py

amarkpayne · 2019-08-09T18:35:58Z

I have force pushed the requested changes and confirmed that the (updated, the species map changed from a dictionary to a list) minimal restart example works.

documentation/source/users/rmg/input.rst

rmgpy/rmg/input.py

mliu49 · 2019-08-09T19:36:21Z

rmgpy/rmg/main.py

+                reordered_core_species = []
+                for spc in restartSpeciesList:
+                    for j, oldCoreSpc in enumerate(self.reactionModel.core.species):
+                        if oldCoreSpc.isIsomorphic(spc):


I think we should use the strict=False argument since we don't generate resonance structures for the restart species.

rmgpy/rmg/main.py

mliu49 · 2019-08-09T19:44:26Z

rmgpy/rmg/main.py


        try:
-            restart = kwargs['restart']
+            self.restart_seed_path = kwargs['restart']


I think this doesn't do anything. It should set the coreSeedPath, edgeSeedPath, filtersPath, and speciesMapPath like you do when loading the input file.

Alternatively, if you don't think we should support both methods, you can remove this and set the -r flag to simply print a warning that the old restart method has been removed and the flag does not do anything.

Fixed, thanks for the catch!

mliu49 · 2019-08-09T23:45:20Z

@amarkpayne was correct, and the problem was in fact the extra indentation, which was messing up the family name parsing and causing the reaction to be loaded as a library reaction with .library (and .family) as seedMechanism.name (here). We missed the warning, which would have led us to the cause much faster.

I added a few commits to resolve this and some related issues:

Switch to using the longDesc property of the entries instead of the private _longDesc attribute in getLibraryReactions. Not a bug, but the point of a private attribute is that it shouldn't be used.
Improve parsing of reaction template and family name in getLibraryReactions, including stripping the family name. I discovered while debugging that the reaction template was completely wrong in some cases because the comment strings weren't exactly as expected.
Remove leading spaces from kinetics comments in getLibraryReactions. I wasn't sure the best place to put this, but it seemed to make sense there. This prevents the gradual increase in indentation with each read/write cycle.

We now perform this check when/if these edge species are moved to the core

Has been deprecated for a while, and is now completely replaced by the restart from seed mechanism feature.

We will still use the job name when outputting to the database, but the name should be fixed in the output directory

More robust methods to determine template and family name Make sure to strip family name

Otherwise, indents are retained over each read/write operation

amarkpayne · 2019-08-10T15:58:47Z

I have also confirmed that with @mliu49 fixes enable unlimited successive restarts. I have also addressed the last remaining comment that I need to fixup, and I have squashed all of the fixups and force pushed. This should be ready for final approval/merging

mliu49

I think this is good to go now. @mjohnson541, do you have any comments on the fixes that I added?

mjohnson541 · 2019-08-11T22:44:43Z

Took a look now, looks fine to me!

Outdated

amarkpayne requested a review from mjohnson541 June 24, 2019 20:33

amarkpayne self-assigned this Jun 24, 2019

amarkpayne added Complexity: Medium Topic: Restart Type: Feature Status: Blocked by PR This PR is dependent on another PR labels Jun 24, 2019

amarkpayne force-pushed the SaveFilterTensors branch 2 times, most recently from 51cdc99 to bf43881 Compare June 25, 2019 01:53

amarkpayne mentioned this pull request Jun 26, 2019

Recursive as dictionary #1643

Merged

amarkpayne force-pushed the SaveFilterTensors branch 2 times, most recently from 47aee7b to 4f73f35 Compare June 28, 2019 21:27

mliu49 reviewed Jul 12, 2019

View reviewed changes

rmgpy/rmg/input.py Outdated Show resolved Hide resolved

amarkpayne force-pushed the SaveFilterTensors branch from 4f73f35 to f25abe5 Compare July 23, 2019 21:17

amarkpayne force-pushed the SaveFilterTensors branch 3 times, most recently from 63a0590 to 47cf386 Compare August 5, 2019 19:58

amarkpayne added Status: Ready for Review PR is complete and ready to be reviewed and removed Status: Blocked by PR This PR is dependent on another PR labels Aug 5, 2019

mliu49 reviewed Aug 6, 2019

View reviewed changes

mliu49 added the Before Py3 Should be merged before Python 3 transition label Aug 6, 2019

amarkpayne commented Aug 6, 2019

View reviewed changes

amarkpayne force-pushed the SaveFilterTensors branch 2 times, most recently from d980302 to 27fd30b Compare August 7, 2019 13:55

mjohnson541 requested changes Aug 7, 2019

View reviewed changes

amarkpayne force-pushed the SaveFilterTensors branch from 27fd30b to 89290b0 Compare August 9, 2019 15:09

mliu49 reviewed Aug 9, 2019

View reviewed changes

mjohnson541 previously requested changes Aug 9, 2019

View reviewed changes

rmgpy/rmg/main.py Outdated Show resolved Hide resolved

rmgpy/rmg/main.py Show resolved Hide resolved

amarkpayne force-pushed the SaveFilterTensors branch from 89290b0 to 325dc8b Compare August 9, 2019 18:34

mliu49 reviewed Aug 9, 2019

View reviewed changes

amarkpayne force-pushed the SaveFilterTensors branch from 325dc8b to 0075b11 Compare August 9, 2019 21:38

amarkpayne and others added 17 commits August 10, 2019 11:24

Save the seed mechanism at the beginning of each iteration

401d544

Make seed mechanism generation every iteration the default behavior

ad8753c

Prevent the loss of the seed mechanism if an exception is encountered

26507fe

Save filter tensors with seed mechanism

840b105

Add flag in RMG input for restarting from a seed mechanism

2763615

Load in a seed mechanism and filter tensors for restarting jobs

b43a547

Do not perform forbidden species checking for auto generated libraries

861a5c2

We now perform this check when/if these edge species are moved to the core

Automatically generate a restart from seed file

97391d9

Add documentation for restarting from a seed mechanism

63caf74

Delete the cPickle restart feature!!!!

93a570f

Has been deprecated for a while, and is now completely replaced by the restart from seed mechanism feature.

Add minimal restart example

d74bdf2

Use reaction filtering in the minimal example

2ee6816

Make the name of the seed mechanism fixed and case consistent

03e4bc6

We will still use the job name when outputting to the database, but the name should be fixed in the output directory

Use degeneracy from a seed mechanism if auto-generated

e601f3f

Use longDesc property instead of private variable

58adee7

Refactor comment parsing for template reactions in kinetics library

f67c64a

More robust methods to determine template and family name Make sure to strip family name

Remove leading spaces from kinetics comments from libraries

9bd9589

Otherwise, indents are retained over each read/write operation

amarkpayne force-pushed the SaveFilterTensors branch from 3a00ee3 to 9bd9589 Compare August 10, 2019 15:56

mliu49 approved these changes Aug 11, 2019

View reviewed changes

mliu49 merged commit 3846fcc into master Aug 12, 2019

mliu49 deleted the SaveFilterTensors branch August 12, 2019 01:19

mliu49 mentioned this pull request Nov 26, 2019

RMG v3.0.0 Release Planning #1830

Closed

mliu49 mentioned this pull request Dec 16, 2019

RMG-Py v3.0.0 Release #1852

Merged

Conversation

amarkpayne commented Jun 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation or Problem

Description of Changes

Testing

Uh oh!

codecov bot commented Jun 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amarkpayne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjohnson541 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mliu49 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjohnson541 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

amarkpayne commented Aug 9, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mliu49 Aug 9, 2019

Choose a reason for hiding this comment

Uh oh!

amarkpayne Aug 10, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mliu49 Aug 9, 2019

Choose a reason for hiding this comment

Uh oh!

amarkpayne Aug 10, 2019

Choose a reason for hiding this comment

amarkpayne commented Jun 24, 2019 •

edited

Loading

codecov bot commented Jun 25, 2019 •

edited

Loading