-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RMG estimates kinetics non-deterministically #494
Comments
This may be relevant: #323. This uses the idea of tracking multiple transition states in a single family. The difficulty lies in generating the reverse direction template. I think we already allow duplicate reactions when they are present in separate reaction families. |
We could:
any other ideas? |
I'm thinking in |
And when estimating kinetics, rxn1's rate should be the sum of rate of template1 and rate of template2 |
@KEHANG your ipython notebook is very valuable as a unit test. Could you convert it into one, and add the WIP decorator onto it so that it passes the travis CI tests. |
Ok, I'll make one. |
@nickvandewiele this is the test I made: 5de3826 |
I made this PR, the test inside is decorated by @work_in_progress. Once we got a solution to the issue, we can turn the decorator off. I will merge the test into master if no more further suggestions. |
@rwest I would probably support solution 2 at least in the case of a single reacting isomer and multiple transition states. However, in the case where there there are multiple resonance forms.... it becomes more complex. Solution 3 is interesting but might be a little too hard to implement. I'm still unsure about Solution 1 and will get back to you on that. |
We could do Solution 1, but choose "fastest at all temperatures" by (a) summing them, which is almost close if they are sufficiently different, or (b) evaluate them all at a bunch of T, find the highest at each T, and re-fit a new modified Arrhenius through these. |
Thanks for the test case @KEHANG. I'm working on a fix for dealing with the kinetics caused by different resonance structures now. If anyone else is already working on a solution, let me know. @rwest. when refitting, I want to avoid losing any data about the templates (and degeneracies of each template) used in the estimation. Max and I have three ideas of how to implement this while keeping the template and degeneracy information. Which one sounds the best to you?
Any thoughts would be helpful. |
Our Is proposal to allow |
I guess the idea behind item 3 was to have separate reaction objects for each non-degenerate transition state path, and not modify the types held by the If option 3 is thrown out, the proposal would be as you suggested: store lists of templates and degeneracies. This would likely require also changing Based on your idea, I also think making the family parameter a list would be beneficial (even though we try to design families to not overlap). It would have helped limit the nondeterministic kinetics caused when the |
closed with #1055 |
The order of molecules in
spc.molecule
can eventually lead to same reactions but different templates matched and therefore different kinetics. This difference could be exposed as modelling diverging if newly developed features have a different molecule order than master branch.I wrote a post using a detailed example at here. I want to take it as a chance to discuss what's the possible ways to solve this bug.
One way to make more deterministic might be redefine the equality of reaction; that is treat reactions with different templates as different reaction although all the species in the reaction are same isomorphically.
The text was updated successfully, but these errors were encountered: