External model solver integration#932
Conversation
There was a problem hiding this comment.
Pull request overview
This PR finalizes an integration API for “external models” (e.g., aerosol/cloud modules) so they can contribute state variables, forcing terms, and Jacobian entries to MICM’s kinetic solvers, while extending ProcessSet to be templated on dense/sparse matrix policies and updating CPU/CUDA call sites accordingly.
Changes:
- Add a new external model integration API (
ExternalModelSystem,ExternalModelProcessSet) and wire it intoSystem,SolverBuilder,Solver, andProcessSet. - Template
ProcessSet(andCudaProcessSet) on dense/sparse matrix policies and update forcing/Jacobian APIs to accept the full solverstate. - Expand/reshape integration and unit/regression tests for the new API, including new aerosol-model integration coverage for both Rosenbrock and Backward Euler.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
test/unit/solver/test_backward_euler.cpp |
Updates test type aliases for newly-templated ProcessSet. |
test/unit/process/test_process_set_policy.hpp |
Updates unit tests for new ProcessSet API (state-first forcing/Jacobian; instance SpeciesUsed). |
test/unit/process/test_process_set.cpp |
Instantiates ProcessSet with explicit dense/sparse policy types in unit tests. |
test/unit/cuda/process/test_cuda_process_set.cpp |
Updates CUDA unit tests for templated CudaProcessSet. |
test/regression/RosenbrockChapman/regression_test_p_force_policy.hpp |
Updates forcing call to pass state first. |
test/regression/RosenbrockChapman/regression_test_dforce_dy_policy.hpp |
Updates Jacobian call to pass state first. |
test/integration/test_aerosol_model_rosenbrock.cpp |
New Rosenbrock-focused aerosol external-model integration test driver. |
test/integration/test_aerosol_model_backward_euler.cpp |
New Backward-Euler-focused aerosol external-model integration test driver. |
test/integration/test_aerosol_model.cpp |
Removes older single aerosol integration test in favor of the new split drivers/policy. |
test/integration/stub_aerosol_1.hpp |
Adds a stub external aerosol model implementation for integration testing. |
test/integration/stub_aerosol_2.hpp |
Adds a second stub external aerosol model with parameter updates and Jacobian/forcing hooks. |
test/integration/aerosol_model_policy.hpp |
Shared policy/helpers used by both new aerosol integration test drivers. |
test/integration/CMakeLists.txt |
Registers the two new integration tests and removes the old one. |
include/micm/system/system.hpp |
Switches System external model storage to ExternalModelSystem and updates sizing/naming. |
include/micm/solver/state.hpp |
Minor comment fix (typo). |
include/micm/solver/solver_builder.hpp |
Adds builder storage + API for external model process sets; updates ProcessSet type aliases. |
include/micm/solver/solver_builder.inl |
Integrates external models into build path: custom parameter map, rates construction, update functions. |
include/micm/solver/solver.hpp |
Stores and invokes external-model “update state parameters” functions during CalculateRateConstants. |
include/micm/solver/rosenbrock.inl |
Updates forcing/Jacobian calls to pass state. |
include/micm/solver/backward_euler.inl |
Updates forcing/Jacobian calls to pass state. |
include/micm/process/process_set.hpp |
Templates ProcessSet, adds external model forcing/Jacobian hooks, and updates APIs accordingly. |
include/micm/external_model.hpp |
New header defining the external model type-erasure wrappers and documentation. |
include/micm/cuda/solver/cuda_solver_builder.hpp |
Updates CUDA builder alias to use templated CudaProcessSet. |
include/micm/cuda/process/cuda_process_set.hpp |
Templates CudaProcessSet, updates signatures to match the new state-first API, and errors on external models. |
include/micm/GPU.hpp |
Updates CUDA solver type aliases to use templated CudaProcessSet. |
include/micm/CPU.hpp |
Updates CPU solver type aliases to use templated ProcessSet. |
docker/Dockerfile.intel |
Adjusts base image reference to docker.io/intel/oneapi-hpckit:latest. |
💡 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 #932 +/- ##
==========================================
+ Coverage 94.29% 94.33% +0.03%
==========================================
Files 68 69 +1
Lines 3263 3321 +58
==========================================
+ Hits 3077 3133 +56
- Misses 186 188 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
K20shores
left a comment
There was a problem hiding this comment.
This looks fantastic! I like how comprehensive the tests are. I don't have any questions as of now
This giant PR finishes defining the API for external models (e.g., aerosol, cloud, etc.), allowing contributions to the forcing and Jacobian calculations. This should be the last major update to MICM for allowing kinetic solves of cloud chemistry. (The rest of the work should be in https://github.com/ncar/miam).
Two external model classes (
ExternalModelSystemandExternalModelProcessSet) are defined insrc/external_model.hpp. The header also includes a description of how to adapt external models to work with the new API.I tried to avoid adding additional template parameters to existing classes, but could not find a practical way to avoid this. The
ProcessSetis now templated withDenseMatrixPolicyandSparseMatrixPolicy, which didn't end up requiring too many code changes (several of the functions already had these template parameters).There were some changes that affected the CUDA code. I modified the code as best I could without access to hardware to test on GPUs. The NVHPC Docker image builds, but the tests will need to be run. The two changes that affect the CUDA code are:
ProcessSettemplate arguments (CudaProcessSetextendsProcessSetand now also includes the same template parameters)AddForcingTermsandSubtractJacobianTermsfunctions now take a state instance as their first argument, instead of just the vector of rate constants.The CUDA solvers don't yet support external models, but should continue to function when external models are not included (or throw a runtime error if they are)