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

KPP to MICM #173

Merged
merged 32 commits into from
Aug 4, 2023
Merged

KPP to MICM #173

merged 32 commits into from
Aug 4, 2023

Conversation

dwfncar
Copy link
Collaborator

@dwfncar dwfncar commented Aug 3, 2023

PR for initial implementation.
Need still to test MICM with the generated JSON from the KPP Chapman example ...

Description:
kpp_to_micm.py translates KPP config files to MICM JSON config files
(desginated by the suffixes .kpp, .spc, .eqn, .def)
from a single directory specified by the --kpp_dir argument.

In the initial implementation,
the KPP sections #ATOMS (not yet used), #DEFFIX (not yet used),
#DEFVAR, and #EQUATIONS are read and parsed.
Equations with the hv reactant are MICM PHOTOLYSIS reactions,
all others are assumed to be ARRHENIUS reactions.

TODO:
(1) Parse both A and B Arrhenius coefficients from KPP equations
Currently a single rate coeffient is assigned to A and B is set to 0.
(2) Translate stoichiometric coefficients in the equation string
with more than one digit.
(3) Add method unit tests with pytest.
(4) Add support for many more reaction types ...

dwfncar and others added 27 commits July 27, 2023 15:44
@dwfncar dwfncar self-assigned this Aug 3, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4c772ed) 96.80% compared to head (acfa4b5) 96.80%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #173   +/-   ##
=======================================
  Coverage   96.80%   96.80%           
=======================================
  Files          24       24           
  Lines        1974     1974           
=======================================
  Hits         1911     1911           
  Misses         63       63           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@mattldawson mattldawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I like the idea of adding some unit tests - this text parsing is always tricky because of all the edge cases

for reactant in reactants:
if reactant[0].isdigit():
equation_dict['reactants'][reactant[1:]] \
= {'yield': float(reactant[0])}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
= {'yield': float(reactant[0])}
= {'qty': float(reactant[0])}

I believe the stoichiometric coefficient on the reactants side is called the quantity, abbreviated to qty, in the configuration format we use.

Comment on lines 211 to 216
parser.add_argument('--micm_species', type=str,
default=os.path.join('..', 'configs', 'micm', 'species.json'),
help='MICM output species config file')
parser.add_argument('--micm_reactions', type=str,
default=os.path.join('..', 'configs', 'micm', 'reactions.json'),
help='MICM output reactions config file')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to change the default output directory to the name of the mechanism, maybe a path of ../configs/micm/<mechanism>? At least this way any overwrite would be for the same mechanism

@dwfncar dwfncar merged commit eb98cae into main Aug 4, 2023
14 of 15 checks passed
@dwfncar dwfncar deleted the kpp_to_micm branch August 4, 2023 16:21
@mattldawson mattldawson added this to the MICM ready for CAT-Chem milestone Oct 11, 2023
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.

6 participants