RMG-Cat!!! #1536
Closed
RMG-Cat!!! #1536
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We start with Liquid because we want constant volume.
This is a combination of commits, that was then rebased manually onto the updated master. These were the original commit messages: Storing Gas mole fractions, and surface coverages (dictionaries) and surface-to-volume ratio and surface site density (floats) Pressure can vary, so only set initialP (contains merged fixup commit, had to resolve conflicts, may be broken) Making surface reactor work (we hope). SpeciesConcentrations are in mol/m3 for gas and mol/m2 for surface. Reaction.getRateCoefficient(T,P) is assumed to return in: m2/mol/s for bimolecular surface reactions m3/mol/s for bimolecular gas phase reactions 1/s for unimolecular reactions (had to modify during rebase) SurfaceReactor stores surfaceVolumeRatio and surfaceSiteDensity as ScalarQuantity objects. These have units, eg. '1/m' or 'mol/m^2' Fixed surface reactor (??)
Ran it through "yapf"
Just asks the first molecule.
This is like a RateCoefficient but with more flexible units. To allow for: A + * -> A* A* -> B* A* + B* -> C* 2A + * + * -> A* + A*
saves the figure instead of opening a window
it has the rmgpy.kinetics.StickingCoefficient class
Important to have __reduce__ return the correct Class, otherwise when you copy or pickle a SurfaceArrhenius it becomes an Arrhenius
This was left over from when it was a simple reactor test
It represents the number of moles in the system.
I find things like
C[j] = N[j] / V
are easier to understand than
C[j] = y[j] / V
The parameters were generated by an RMG-cat simulation. If we replace the StickingCoefficient expression with a SurfaceArrhenius expression (currently commented out) then it works. As it is, it crashes the ODE solver. Conclusion: the StickingCoefficient code is broken.
…at Richard had already included them further down in the file.
Unfortunately this introduces more reaction-family name hard-coding, but for now it seems to work, and I couldn't quickly think of a more elegant solution.
Rather than use the recursive functional group drawing code, just generate the coordinates as best as possible, then rotate the whole molecule.
Not sure what colour to use. Some weird purple for now :-/
See cfgoldsmith#5 for more detailed version.
This should stop "desorption" of one R-X bond of a bidentate species making a desorbed species that is still adsorbed. Once working, probably want to quieten down the logging on line 1594. Hopefully closes cfgoldsmith#7
Not sure how to get the + signs in a nicer place.
Unsure if this has consequences on flux diagrams or sensitivity analyses, etc. but I think it's a helpful start towards making more sense of simulation outputs, especially for two-phase heterogeneous systems.
The idea is that if the object has units of 'm^3/mol/s' then but you want to get it in chemkin format you need the value in 'cm^3/mol/s'. This gives the factor required.
The problem was that O=X (where X is the surface site) was forming [O+]#X which has a net charge. Not sure if this is the best way to solve the problem, but it solves the problem. Closes cfgoldsmith#43
The new syntax requires specifying different sets of reaction families.
…model. I forgot to fix the unit test too.
These take ages to load.
This way of doing things is a bit fragile, and not very satisfying. But I think this fixes at least a couple of places where this change needs to be made.
Now passes last unit test. (It was something like 0.1000000000000149)
…fficient and also they can be converted into BEP equivalents from training data.
Some reaction families have two surface sites, making them termolecular in a way, but with only two reactants. A + X + X --> Combining this with genuine termolecular reaction families, makes it a tad confusing.
Replaces isSurfaceSpecies()
Must specify 'eV/molecule'
Following the instructions at https://github.com/ReactionMechanismGenerator/RMG-Py/wiki/Simultaneous-Update-of-RMG-Py-and-RMG-database This commit should be removed before merging to master.
Merged
|
Replaced by #1537 which comes from the ReactionMechanismGenerator fork rather than the rwest fork. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm hopefully going to continue trying to tidy up this pull request via some rebasing to make it easier to review in chunks, but however this is sliced up in terms of commits, I think this PR contains all the actual changes that will be included in merging RMG-Cat, so if anyone brave wants to start reviewing, here at least is something we can start to discuss...
(Oh, it needs the cat version of the RMG-database too)