-
Notifications
You must be signed in to change notification settings - Fork 7
Configure aerosol-phase-specific Species for solver #891
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds aerosol-phase-specific scoping functionality to the chemical reaction builder, allowing species names to be prefixed with aerosol mode and phase identifiers. This enables the solver to distinguish between the same chemical species in different aerosol phases and modes.
Key Changes:
- Introduced
SetAerosolScopemethod to enable automatic scoping of reactant and product species names - Modified
SetReactantsandSetProductsmethods to apply scoping when enabled - Added comprehensive test coverage for the new scoping functionality
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
test/unit/process/test_aerosol_scope.cpp |
New test file with 4 test cases covering various scoping scenarios including single/multiple reactants and products |
test/unit/process/CMakeLists.txt |
Registers the new test file in the build system |
include/micm/process/chemical_reaction_builder.hpp |
Implements the aerosol scoping functionality with new methods and modified builder pattern methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #891 +/- ##
==========================================
+ Coverage 94.36% 94.64% +0.27%
==========================================
Files 65 65
Lines 3197 3231 +34
==========================================
+ Hits 3017 3058 +41
+ Misses 180 173 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
K20shores
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why we need the scope. The issue mentioned in #888 was that a species in a phase couldn't be found and said nothing about a scope. Wouldn't this be fixed only by updating the species name?
species.name_ = phase.name_ + "." + species.name_;
When a species like
CO2exists in multiple modes/sections, the current MICM API cannot handle which instance it needs to react.This PR:
For example,
CO2can exist in gas phase, or in a mode/section-phase-specific form.