Skip to content
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

Adding in coverage dependent effects to surface kinetics #2090

Merged
merged 43 commits into from Jul 23, 2021
Merged

Adding in coverage dependent effects to surface kinetics #2090

merged 43 commits into from Jul 23, 2021

Commits on Jul 23, 2021

  1. Adding in coverage parameters to SurfaceArrhenius

    Adding coverage dependent parameters to surface arrhenius kinetics
    added in coverage dependence in kinetics model
    Adding _coverage_dependence to SurfaceArrhenius and SurfaceArrhenius Cython declarations
    adding variable names to coverage documentation and making it more readable
    Fixing the updated docstrings for coverage_dependence
    
    Adding in coverage_dependence to SurfaceArrhenius and SurfaceArrheniusBEP
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    e099db7 View commit details
    Browse the repository at this point in the history
  2. Use coverage parameters to modify the reaction rates

    using coverage dependent parameters
    
    multiply kr by cov parameters
    
    moving cov_dep and co to the init
    
    WIP: Export cantera files with coverage dependent parameters
    
    make cov dict into cantera readable format for export
    
    WIP: Adding unittests to test cantera export
    
    Make sure units of coverage dependence are si
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    21a4da7 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    9cf4424 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    548ce1c View commit details
    Browse the repository at this point in the history
  5. Adding in new unit tests to the solver surface test

    test that coverage dependence is doing what it should be doing
    
    Richard: Fix some surface solver tests details.
    This is meant to represent CH3X, not HX.
    Doesn't make a difference to what we're testing, but
    better to be right.
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    3494321 View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    18a1810 View commit details
    Browse the repository at this point in the history
  7. Adding in coverage dependent database tests

    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    bb2c693 View commit details
    Browse the repository at this point in the history
  8. Add species labels in reactionTest

    ChrisBNEU authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    b0be206 View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    a16a2be View commit details
    Browse the repository at this point in the history
  10. Changing coverage dependence SMILES to species objects

    Look for coverage_dependence attribute
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    35c7b0b View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    a198243 View commit details
    Browse the repository at this point in the history
  12. Configuration menu
    Copy the full SHA
    2fcea33 View commit details
    Browse the repository at this point in the history
  13. use species labels in the __repr__

    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    f62cedf View commit details
    Browse the repository at this point in the history
  14. Configuration menu
    Copy the full SHA
    b683d1b View commit details
    Browse the repository at this point in the history
  15. Can read Coverage Dependent kinetics from Chemkin files

    For now, only reactions with COV are turned into SurfaceArrhenius objects,
    all others are left as Arrhenius. 
    I'm not quite sure what's happening around lines 467-473
    (Emily's changes, I'm just committing them)
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    268a3e6 View commit details
    Browse the repository at this point in the history
  16. Configuration menu
    Copy the full SHA
    9cdd49b View commit details
    Browse the repository at this point in the history
  17. Do not write coverage dependent parameters if coverage dependence is …

    …turned off
    
    save_chemkin_file has coverage_dependence flag, which is passed on to write_kinetics_entry
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    127d725 View commit details
    Browse the repository at this point in the history
  18. Adding in chemkin test to check we can read and write coverage depend…

    …ent parameters
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    1c2d51d View commit details
    Browse the repository at this point in the history
  19. Turn on coverage dependence in the coverage-dependence solver tests.

    This defaults to False if not specified as true.
    The fact that this wasn't noticed, tells us that the test isn't really
    testing that we do evaluate coverage-dependence correctly.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    884c4e5 View commit details
    Browse the repository at this point in the history
  20. Correcting ordering of COV parameters in Chemkin files.

    Should be (a, m, E).
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    93979ea View commit details
    Browse the repository at this point in the history
  21. Chemkin reading COV: Do parameters in order a, m, E.

    also, turning the forgotten 'J/mol' into Eunits !
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    9bb42c1 View commit details
    Browse the repository at this point in the history
  22. Configuration menu
    Copy the full SHA
    9e17c6e View commit details
    Browse the repository at this point in the history
  23. Chemkin reading COV parameters: should preserve case of species names.

    If we're even going to bother writing this code we really ought to test it
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    fec8618 View commit details
    Browse the repository at this point in the history
  24. Put coverage-dependence paramaters in order a, m, E, consistently.

    Dictionaries are order-preserving, and humans' brains are, so we'll have
    fewer bugs if we're consistent with ourselves and other software's conventions.
    Chemkin and Cantera use (a, m, E) ordering. Now we do, throughout.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    e670cee View commit details
    Browse the repository at this point in the history
  25. Chemkin reading COV parameters. Fixes for reading sticking coefficients.

    If there's a STICK line before a COV line we can cope.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    27978dd View commit details
    Browse the repository at this point in the history
  26. Chemkin COV writing: correct units for the E parameter

    The E should be in the same units as Ea, which is kcal/mol.
    That's 4184 times smaller.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    ad508c9 View commit details
    Browse the repository at this point in the history
  27. update __repr__ string writing

    do not write if the coverage dependence dictionary is empty
    mazeau authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    b12ded6 View commit details
    Browse the repository at this point in the history
  28. Revert "Do not write coverage dependent parameters if coverage depend…

    …ence is turned off"
    
    This reverts commit 7841f24.
    Had to manually tweak a few things to resolve conflicts.
    
    The point is to remove the extra code from the chemkin-writing methods.
    If we turn off coverage, the parameters should not be in the model.
    If the parameters are in the model, they should be written by chemkin.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    dd5f119 View commit details
    Browse the repository at this point in the history
  29. Chemkin writing coverage dependence: fix bugs

    - An empty dictionary shouldn't be printed (even though it's not 'None')
    - We need sticking coefficients cov parameters to be in the right units too!
      (we previously fixed from J/mol to kcal/mol in one place, but not both)
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    61f7c88 View commit details
    Browse the repository at this point in the history
  30. Chemkin writing coverage dependence: reduce code duplication

    Protects us from bugs where we fix things in one place and not the other.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    78f87d8 View commit details
    Browse the repository at this point in the history
  31. Renaming apply_coverage_depndence_to_reaction to process_coverage_dep…

    …endence.
    
    This better describes what it does. Also expanded docstring.
    Also removed it from INSIDE the apply_kinetics_to_reaction method
    and removed if from and elif block, so that it is duplicated
    in less code (but still called in the same way, I think)
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    c109911 View commit details
    Browse the repository at this point in the history
  32. process_coverage_dependence now REMOVES parameters if turned off.

    I'm still not very sure about the `coverage_dependence=False` 
    settings, and what should be turned off by it.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    6494c17 View commit details
    Browse the repository at this point in the history
  33. SurfaceReactor: Remove unused (and mutable) default arguments.

    Putting dictionaries as a default argument is a famous gotcha
    https://docs.python-guide.org/writing/gotchas/
    I wondered if it's being done on purpose for some efficiency
    reason, to share memory, but decided no. a risk of error,
    and is not needed anyway as we never initialize the object
    with those parameters anyway.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    bc99a4e View commit details
    Browse the repository at this point in the history
  34. CovDep test: report timing of the simulations.

    I'm hoping to speed it up later.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    7eda4c3 View commit details
    Browse the repository at this point in the history
  35. CovDep test: plot graphs.

    They look the same :-/
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    6dee145 View commit details
    Browse the repository at this point in the history
  36. Configuration menu
    Copy the full SHA
    7425b4e View commit details
    Browse the repository at this point in the history
  37. CovDep test: change T, and plot coverage.

    At 600 K the eqm coverage is much higher, so you get
    a more noticeable coverage dependence.
    
    By plotting coverage instead of concentration it's 
    easier to see what's happening.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    bbbba5e View commit details
    Browse the repository at this point in the history
  38. CovDep Test: make the surface reactor test slower.

    Add a bunch of redundant species and reactions, to slow it down 
    a bit, for benchmarking purposes.
    
    Simulation took 3.550e+00 seconds in rmgpy.solver.surfaceTest.SurfaceReactorCheck.test_solve_h2_coverage_dependence
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    4670244 View commit details
    Browse the repository at this point in the history
  39. Refactor the coverage dependence calculations in surface solver.

    Main goal was to optimize everything happening inside the 
    loops in the "residual" function (which gets called a lot).
    
    Simulation took 4.541e-01 seconds in rmgpy.solver.surfaceTest.SurfaceReactorCheck.test_solve_h2_coverage_dependence
    
    (previous run was 3.550e+00, so we're about 8x faster)
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    cf8146b View commit details
    Browse the repository at this point in the history
  40. CovDep test: stop plotting the graphs.

    These are helpful for manual inspection and helped me confirm
    it's working, but they're not needed for the automated test 
    suite or continuous integration.
    rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    fa20dea View commit details
    Browse the repository at this point in the history
  41. added check that covdep makes a difference

    changed CH3 coverage unittest to run an additional simulation without coverage dependence. the solution arrays from each are compared.
    ChrisBNEU authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    a00934b View commit details
    Browse the repository at this point in the history
  42. updated documentation with coverage dependence

    added instructions on input file construction and updating the database.
    ChrisBNEU authored and rwest committed Jul 23, 2021
    Configuration menu
    Copy the full SHA
    99ab281 View commit details
    Browse the repository at this point in the history
  43. Configuration menu
    Copy the full SHA
    5918981 View commit details
    Browse the repository at this point in the history