Skip to content

Conversation

@boulderdaze
Copy link
Collaborator

@boulderdaze boulderdaze commented Jan 9, 2026

Waiting for #891 to be reveiwed/merged

Systems.others_ is intended to store the data that are not species, such as mode specific number concentration. But because it is implemented as a map, for example, mode-specific density can not be stored alongside number concentration.
This PR replaces the map with a vector, removing key-pair restriction.

// current
system.others_[AEROSOL.AITKEN] = NUMBER_CONCENTRATION
system.others_[AEROSOL.AITKEN] = DENSITY

// this PR
AEROSOL.AITKEN.NUMBER_CONCENTRATION
AEROSOL.AITKEN.DENSITY

Copy link
Contributor

Copilot AI left a 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 refactors System.others_ from a map to a vector structure to support storing multiple properties (e.g., number concentration and density) for the same aerosol mode. Additionally, it introduces aerosol scoping functionality in ChemicalReactionBuilder to automatically prefix species names with mode and phase identifiers.

Key changes:

  • Changed System.others_ from std::unordered_map<std::string, std::string> to std::vector<std::string>
  • Added JoinStrings utility function for concatenating dot-separated names
  • Introduced SetAerosolScope method in ChemicalReactionBuilder with mutual exclusivity enforcement

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
include/micm/system/system.hpp Changed others_ from map to vector, updated UniqueNames() and StateSize() methods, added JoinStrings usage
include/micm/util/utils.hpp Added new JoinStrings utility function for joining strings with dot separators
include/micm/process/chemical_reaction_builder.hpp Added SetAerosolScope method with scoping logic, updated SetReactants and SetProducts to support scoping
include/micm/process/process_error.hpp Added InvalidConfiguration error code and updated error message ordering
include/micm/util/error.hpp Added MICM_PROCESS_ERROR_CODE_INVALID_CONFIGURATION constant
test/unit/system/test_system.cpp Updated test to use vector-based API with emplace_back
test/unit/solver/test_state.cpp Updated tests to use vector-based API with push_back
test/unit/process/test_process_configuration.cpp Added comprehensive tests for aerosol scoping functionality
test/unit/process/CMakeLists.txt Registered new aerosol scope test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link

codecov-commenter commented Jan 9, 2026

Codecov Report

❌ Patch coverage is 94.02985% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.66%. Comparing base (33e7f7a) to head (9147b0d).

Files with missing lines Patch % Lines
include/micm/process/process_error.hpp 33.33% 2 Missing ⚠️
include/micm/process/chemical_reaction_builder.hpp 97.36% 1 Missing ⚠️
include/micm/util/utils.hpp 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #894      +/-   ##
==========================================
+ Coverage   94.36%   94.66%   +0.29%     
==========================================
  Files          65       66       +1     
  Lines        3197     3245      +48     
==========================================
+ Hits         3017     3072      +55     
+ Misses        180      173       -7     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants