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

Catch missing RO2 species error when prepping #457

Merged
merged 2 commits into from
Aug 3, 2021
Merged

Catch missing RO2 species error when prepping #457

merged 2 commits into from
Aug 3, 2021

Conversation

kilicomu
Copy link
Contributor

@kilicomu kilicomu commented Aug 3, 2021

Adds an exception where an RO2 species is encountered in the RO2 sum of the mechanism file but isn't encountered in the mechanism species list.

We shouldn't get to a point where there is an RO2 species in mechanism.ro2 that isn't in the full species list in the mechanism, so I think the user needs to correct their mechanism before building and running the model, as opposed to a warning being shown and them being allowed to erroneously continue.

I had a look at adding some error handling to the ro2Sum function in outputFunctions.f90 - it is a pure function which disallows error stop. I think there would be a bit too much unpicking to make that error handling neat, so I avoided it.

Let me know what you think - happy to make changes as requested.

Closes #453

Adds an exception where an RO2 species is encountered in the RO2 sum of
the mechanism file but isn't encountered in the mechanism species list.

We shouldn't get to a point where there is an RO2 species in
mechanism.ro2 that isn't in the full species list in the mechanism, so
the user needs to correct their mechanism file before running the model!
@codecov
Copy link

codecov bot commented Aug 3, 2021

Codecov Report

Merging #457 (283e1b6) into master (aeffeb2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #457   +/-   ##
=======================================
  Coverage   65.51%   65.51%           
=======================================
  Files          17       17           
  Lines        2047     2047           
=======================================
  Hits         1341     1341           
  Misses        706      706           
Flag Coverage Δ
build 52.23% <ø> (ø)
tests 82.81% <ø> (ø)
unittests 31.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aeffeb2...283e1b6. Read the comment docs.

Copy link
Collaborator

@spco spco left a comment

Choose a reason for hiding this comment

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

Generally looks good (see comment).

@@ -463,7 +463,15 @@ def convert(input_file, mech_dir, mcm_dir):
# This code only executes if the break is NOT called, i.e. if the loop runs to completion without the RO2 being
# found in the species list
else:
ro2_file.write('0 ! error RO2 not in mechanism: ' + ro2List_i + '\n')
error_message = ' ****** Warning: RO2 species "' + str(ro2List_i.strip()) + '" NOT found in the mechanism. Please check the RO2 section of your mechansm file for incorrect species names! ******'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this line has snuck into the commit and needs removing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep you're right - relic from messing around.

I've removed that line.

@spco spco merged commit dd25104 into AtChem:master Aug 3, 2021
@kilicomu kilicomu deleted the bugfix/catch_missing_ro2 branch August 3, 2021 14:33
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.

Array indexing error via mechanism.ro2
2 participants