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

Fix species identification in reaction wells when generating flux diagrams #145

Merged
merged 6 commits into from
Jan 25, 2024

Conversation

alongd
Copy link
Member

@alongd alongd commented Nov 28, 2023

Properly count species in cases where a species label is contained within another species label, e.g., NO and NO2. This is avoided when using RMG labels like S(1243), but is not avoided when generating flux diagrams for non-RMG generated Cantera models.
Tests added.

Also added additional features to the flux diagram generator, such as allowed_species to only show specific species of interest, and whether to report the actual flux or the relative flux on the arrows.

@alongd alongd self-assigned this Nov 28, 2023
@github-actions github-actions bot added the tests label Nov 28, 2023
@alongd alongd force-pushed the flux_d_fixes branch 3 times, most recently from c2360d1 to b849c19 Compare November 29, 2023 06:06
@alongd alongd requested a review from calvinp0 November 29, 2023 06:41
e.g., if the concentrations were fed incorrectly
To properly count species in cases where a species label is contained within another species label, e.g., 'NO' and 'NO2'. This is avoided when using RMG labels like 'S(1243)', but is not avoided when generating flux diagrams for non-RMG generated Cantera models
To create edges only between a list of selected nodes if desired
@codecov-commenter
Copy link

Codecov Report

Attention: 81 lines in your changes are missing coverage. Please review.

Comparison is base (2de3b76) 73.46% compared to head (8ac4929) 74.91%.
Report is 84 commits behind head on main.

Files Patch % Lines
t3/utils/flux.py 81.42% 52 Missing and 13 partials ⚠️
t3/utils/fix_cantera.py 90.66% 3 Missing and 4 partials ⚠️
t3/simulate/rmg_constantTP.py 0.00% 5 Missing ⚠️
t3/imports.py 40.00% 3 Missing ⚠️
t3/runners/rmg_runner.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
+ Coverage   73.46%   74.91%   +1.44%     
==========================================
  Files          22       24       +2     
  Lines        2891     3317     +426     
  Branches      762      853      +91     
==========================================
+ Hits         2124     2485     +361     
- Misses        552      601      +49     
- Partials      215      231      +16     
Flag Coverage Δ
unittests 74.91% <81.46%> (?)

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

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

Copy link
Member

@JintaoWu98 JintaoWu98 left a comment

Choose a reason for hiding this comment

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

LGTM!

@JintaoWu98 JintaoWu98 merged commit 868e2de into main Jan 25, 2024
4 checks passed
@JintaoWu98 JintaoWu98 deleted the flux_d_fixes branch January 25, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants