Skip to content

Fix bug in disproportionation tree#159

Merged
nyee merged 1 commit intomasterfrom
dispropBug
Feb 3, 2017
Merged

Fix bug in disproportionation tree#159
nyee merged 1 commit intomasterfrom
dispropBug

Conversation

@mliu49
Copy link
Copy Markdown
Contributor

@mliu49 mliu49 commented Jan 17, 2017

H_rad is child of Y_rad, so it should not be listed in the top logic node. Also move H_rad higher in the tree since it's a common radical.

H_rad was added to the logic node in 155821c, probably by accident, It is still listed as a child of Y_rad in the tree structure.

Addresses #158.

@mliu49 mliu49 requested a review from nyee January 17, 2017 23:21
@nyee
Copy link
Copy Markdown
Contributor

nyee commented Jan 18, 2017

One quick check on the RMG-tests. I notice that after the change many disprop reactions involving H are no longer "in the model" according the the RMG-test. Can you double check that these reactions are still created, and that the difference is really just a degeneracy?

@mliu49
Copy link
Copy Markdown
Contributor Author

mliu49 commented Jan 18, 2017

I checked a bunch of them, and the reactions can still be found, with half the degeneracy they had previously.

However, it's not clear to me why they would disappear from the edge with a rate change. I thought the edge included all the reactions between core species? The number of core and edge species were the same between the test and benchmark.

@nyee
Copy link
Copy Markdown
Contributor

nyee commented Jan 21, 2017

Somehow on the line of 1321 of RMG-tests build, the MHC has fails observables without any differences in thermo or kinetics. @KEHANG @mliu49 anybody have any idea what is going on?

H_rad is child of Y_rad, so it should not be listed in the top logic node.
Also move H_rad higher in the tree since it's a common radical.
@nyee nyee merged commit 819b46d into master Feb 3, 2017
@nyee nyee deleted the dispropBug branch February 3, 2017 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants