Network Explorer Part 2 #1545

mjohnson541 · 2019-02-02T21:20:55Z

This PR improves the network explorer tool. It enables use of bimolecular reactant channels and changes the definition of explore_tol. Before explore_tol was an explicit rate coefficient the leak flux of the network was compared to. This PR uses the sum of the coefficients for the net reactions from the source to all other channels to scale the leak rate coefficient. This means that the network will be expanded if the leak rate of the network is larger than explore_tol times the characteristic rate coefficient. Where the charactersitic rate coefficient is the sum of the rate coefficients for all (net) reactions taking the source channel to any other channel in the network.

Check that the unittests pass, run the methoxy network example and take a look at how the new explore_tol affects network generation.

codecov · 2019-02-06T01:18:25Z

Codecov Report

Merging #1545 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1545   +/-   ##
=======================================
  Coverage   41.65%   41.65%           
=======================================
  Files         166      166           
  Lines       28452    28452           
  Branches     5855     5855           
=======================================
  Hits        11853    11853           
  Misses      15777    15777           
  Partials      822      822

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 0db89ac...e09ad75. Read the comment docs.

mjohnson541 · 2019-02-06T21:45:34Z

This should be ready for review now.

alongd

Looking good! See some comments

alongd · 2019-03-25T13:28:04Z

arkane/explorer.py

+            flags = np.array([s.molecule[0].getFormula()==form for s in reaction_model.core.species])
+            biflags = np.zeros((len(reaction_model.core.species),len(reaction_model.core.species)))
+        elif len(self.source) == 2:
+            flags = np.array([s.molecule[0].getFormula()==form for s in reaction_model.core.species])


If flags is being set the same regardless of the condition, perhaps set it before the if statement? Or should it be set to zeroes if len(self.source)==2?

alongd · 2019-03-25T13:33:59Z

arkane/explorer.py

-                    if network.getLeakCoefficient(T=T,P=P) > self.explore_tol:
+                    kchar = 0.0 #compute the characteristic rate coefficient by summing all rate coefficients from the reactant channel
+                    for rxn in self.network.netReactions:#reaction_model.core.reactions+reaction_model.edge.reactions:
+                        if set(rxn.reactants) == set(self.source) and len(rxn.products) == 1 and rxn.products[0].molecule[0].getFormula() == form:


Can we have len(rxn.products) == 1 but not rxn.products[0].molecule[0].getFormula() == form?

Do we need to check rxn.products[0].molecule[0].getFormula() == form as well in this condition? Shouldn't checking for len(rxn.products) == 1 be enough? If there's only one product, can it have a different formula than the PES?

alongd · 2019-03-25T13:39:57Z

arkane/explorer.py

-                        if set(rxn.reactants) == set(self.source) and len(rxn.products) == 1 and rxn.products[0].molecule[0].getFormula() == form:
-                            kchar += rxn.kinetics.getRateCoefficient(T=T,P=P)
-                        elif set(rxn.products) == set(self.source) and len(rxn.reactants) == 1 and rxn.reactants[0].molecule[0].getFormula() == form:
-                            kchar += rxn.generateReverseRateCoefficient(network_kinetics=True).getRateCoefficient(T=T,P=P)


I think this chunk was added in a commit which is part of this PR:
use the sum of the rate coefficients for all net rates from the, f236722

Can you merge these instead of deleting the code?

I think this is fixed too.

alongd · 2019-03-25T13:47:27Z

arkane/mainTest.py


    def test_arkane_examples(self):
        for example_type in self.example_types:
            example_type_path = os.path.join(self.base_path, example_type)
-            for example in os.listdir(example_type_path):
+            for example in sorted(os.listdir(example_type_path)):


Can you check whether #1565 solved this problem?

It doesn't solve the issue

mjohnson541 · 2019-04-09T22:04:45Z

I think this should be ready.

…annel

…e channel to every other channel times the explore_tol for the threshold before explore_tol was simply an unscaled threshold for the leak rate

…ore_tol

this results in multiple output files

…s that produces two networks

…ad of explorerjob.network

So it appears that the order of the explorer/network examples matters significantly enough that some orders give you database loading related errors The logic of the errors and the ordering aren't obvious for this reason the explorer examples are run before the network examples and the the os.listdir() output is sorted to maintain consistency

… files

alongd

Looking good. See one minor comment. I'd like to test it out before we merge

alongd · 2019-04-14T14:54:29Z

arkane/explorer.py

-                    if network.getLeakCoefficient(T=T,P=P) > self.explore_tol:
+                    kchar = 0.0 #compute the characteristic rate coefficient by summing all rate coefficients from the reactant channel
+                    for rxn in self.network.netReactions:#reaction_model.core.reactions+reaction_model.edge.reactions:
+                        if set(rxn.reactants) == set(self.source) and len(rxn.products) == 1 and rxn.products[0].molecule[0].getFormula() == form:


Do we need to check rxn.products[0].molecule[0].getFormula() == form as well in this condition? Shouldn't checking for len(rxn.products) == 1 be enough? If there's only one product, can it have a different formula than the PES?

alongd · 2019-04-14T15:07:15Z

@mjohnson541, I reviewed by commits, I see that the final code doesn't include the line I commented on. Please ignore my comment above. Maybe you'd like to polish the commits so there won't be additions and deletions of the same code blocks in a single PR?

mjohnson541 · 2019-04-14T15:11:11Z

I could do that, but the order in which the commits were applied wouldn't make sense any longer. It would mean applying two major changes at once in one commit, which is a lot harder to read than the current way (even if its a bit more difficult to review).

mjohnson541 · 2019-04-22T22:03:23Z

@alongd pdf overwritting issue is fixed can you merge?

alongd

Perfect! Sorry for being too slow.

mjohnson541 added Arkane Status: Ready for Review PR is complete and ready to be reviewed Complexity: Low labels Feb 2, 2019

mjohnson541 self-assigned this Feb 2, 2019

mjohnson541 requested a review from alongd February 2, 2019 21:20

mjohnson541 changed the title ~~Network Exploration Part 2~~ Network Explorer Part 2 Feb 2, 2019

mjohnson541 force-pushed the explorer2 branch 3 times, most recently from ea62530 to 9153c83 Compare February 6, 2019 20:52

mjohnson541 force-pushed the explorer2 branch from 25869a1 to 26419b1 Compare February 23, 2019 21:21

alongd reviewed Mar 25, 2019

View reviewed changes

mjohnson541 force-pushed the explorer2 branch from 26419b1 to ccbfef9 Compare April 9, 2019 19:31

mjohnson541 added 13 commits April 11, 2019 15:22

make it possible to generate a network from a bimolecular reactant ch…

6d4dd3e

…annel

make explore_tol a dimensionless scalar instead of a rate coefficient

e555121

fix bug with network molecular formula determination

52bed57

use the sum of the rate coefficients for all net rates from the sourc…

1ddfc39

…e channel to every other channel times the explore_tol for the threshold before explore_tol was simply an unscaled threshold for the leak rate

switch the methoxy example to using dimensionless explore_tol

368f5e6

update the documentation to reflect changes in the definition of expl…

6c78949

…ore_tol

update the file used in explorerTest.py to use a valid explore_tol value

fa64d24

analyze multiple networks if generated initially

6f39eb0

this results in multiple output files

Add explorer example that involves a source channel with two reactant…

55ce749

…s that produces two networks

adjust the explorer tests so that they use explorerjob.networks inste…

87a330d

…ad of explorerjob.network

separate explorer examples from network examples

3f86bc2

Write comments with thermo and reaction sources to Arkane pdep output…

d580928

… files

mjohnson541 force-pushed the explorer2 branch from ccbfef9 to d580928 Compare April 11, 2019 19:22

alongd reviewed Apr 14, 2019

View reviewed changes

rename network.pdf's as they are made so they don't overwrite each other

e09ad75

mjohnson541 force-pushed the explorer2 branch from 5d09105 to e09ad75 Compare April 22, 2019 19:39

alongd approved these changes Apr 23, 2019

View reviewed changes

alongd merged commit 88e62a8 into master Apr 23, 2019

alongd deleted the explorer2 branch April 23, 2019 00:04

alongd removed the Status: Ready for Review PR is complete and ready to be reviewed label Apr 23, 2019

mliu49 mentioned this pull request May 15, 2019

RMG v2.4.0 (?) Release Planning #1591

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Network Explorer Part 2 #1545

Network Explorer Part 2 #1545

mjohnson541 commented Feb 2, 2019

codecov bot commented Feb 6, 2019 •

edited

Loading

mjohnson541 commented Feb 6, 2019

alongd left a comment

alongd Mar 25, 2019

mjohnson541 Apr 9, 2019

alongd Mar 25, 2019

mjohnson541 Apr 9, 2019

alongd Apr 14, 2019

alongd Mar 25, 2019

mjohnson541 Apr 9, 2019

alongd Mar 25, 2019

mjohnson541 Apr 9, 2019

mjohnson541 commented Apr 9, 2019

alongd left a comment

alongd Apr 14, 2019

alongd commented Apr 14, 2019

mjohnson541 commented Apr 14, 2019 •

edited

Loading

mjohnson541 commented Apr 22, 2019

alongd left a comment

Network Explorer Part 2 #1545

Network Explorer Part 2 #1545

Conversation

mjohnson541 commented Feb 2, 2019

codecov bot commented Feb 6, 2019 • edited Loading

Codecov Report

mjohnson541 commented Feb 6, 2019

alongd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjohnson541 commented Apr 9, 2019

alongd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alongd commented Apr 14, 2019

mjohnson541 commented Apr 14, 2019 • edited Loading

mjohnson541 commented Apr 22, 2019

alongd left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 6, 2019 •

edited

Loading

mjohnson541 commented Apr 14, 2019 •

edited

Loading