Created a new reactor type MBSampledReactor for modeling laser lab Mass-Spec experiments#1669
Created a new reactor type MBSampledReactor for modeling laser lab Mass-Spec experiments#1669
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1669 +/- ##
==========================================
+ Coverage 41.75% 41.87% +0.11%
==========================================
Files 176 177 +1
Lines 29423 29490 +67
Branches 6075 6097 +22
==========================================
+ Hits 12286 12349 +63
- Misses 16261 16262 +1
- Partials 876 879 +3
Continue to review full report at Codecov.
|
|
I think it would make sense for MBSampledReactor to be a subclass of SimpleReactor. I compared the two, and it seemed like only the I added a commit making this change, but have not tested it yet beyond checking that it compiled correctly. Could you run a simulation to see if this works and gives the same result as before? |
|
Update regarding my previous commit. Turns out that it won't work because SimpleReactor defines a |
mliu49
force-pushed
the
simulate_MB_sampled_reactor
branch
from
40ff482 to
0bccf72
Compare
For modeling molecular beam sampling of species from an isothermal, isobaric, homogenous batch reactor. The sampling process is modeled as two first-order unimolecular reactions, following the approach developed by Baeza-Romero et al., in their 2011 IJCK paper. Currently, the new reactor is only intended for use with the simulate.py script, after a conventional RMG mechanism has already been made.
mliu49
force-pushed
the
simulate_MB_sampled_reactor
branch
from
0bccf72 to
42dae5e
Compare
amarkpayne
left a comment
amarkpayne
left a comment
There was a problem hiding this comment.
I have looked over the unit test portions as requested. There is just one quick fix, but otherwise this looks good. I am about to test that the tests run properly by running locally, but go ahead and rebase with the fix squashed.
|
When I run the test locally, this is the plot that I get. Is this what the simulation is supposed to do?
|
mliu49
force-pushed
the
simulate_MB_sampled_reactor
branch
from
2b2fe62 to
206f149
Compare
|
Yeah, the concentrations are all extremely low, so they don't show up on the plot. |
amarkpayne
left a comment
amarkpayne
left a comment
There was a problem hiding this comment.
The unit test commits are approved as far as I am concerned. If the reactor commit is fine with you then this PR can be merged
mliu49
left a comment
mliu49
left a comment
There was a problem hiding this comment.
To document offline discussion, we decided that even though the MBSampledReactor is not up to date with all of the changes to SimpleReactor, it's ok because it does not need to be used in RMG jobs. The workflow currently used is to generate an RMG model using SimpleReactor and then use MBSampledReactor to simulate and compare to the experiments.
4 participants
Motivation or Problem
To model the sampling delay in mass-spec of Green group laser apparatus, it is important to generate a new reactor type.
Description of Changes
New reactor type added, and can be used in simulate.py
Testing
The MBSampledReactor added works same as what it was before rebase on master. Same time-dependent concentration for species by giving same input files
Reviewer Tips