Skip to content

Add emissions configuration schema (inventories, species maps, sources) to mechanism v1#281

Merged
smeechncar merged 7 commits into
mainfrom
feature/emissions-v1
Jul 2, 2026
Merged

Add emissions configuration schema (inventories, species maps, sources) to mechanism v1#281
smeechncar merged 7 commits into
mainfrom
feature/emissions-v1

Conversation

@smeechncar

@smeechncar smeechncar commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #272.

Adds a new emissions::v1 namespace to MechanismConfiguration that
parses YAML-based emissions configs for consumption by MUSICA/MIEM.

What's new

  • include/mechanism_configuration/emissions/v1/types.hpp — public types
    (EmissionsConfig, SourceDescriptor, Inventory, SpeciesMap, etc.)
  • include/mechanism_configuration/emissions/v1/parse.hpp — public API:
    ParseEmissions(path) and ParseEmissions(string)
  • src/emissions/v1/parser.cpp — full implementation (yaml-cpp private,
    not exposed in headers); supports ${VAR} / ${VAR:-default} env var
    expansion in data root
  • 12 new ErrorCode values in errors.hpp / errors.cpp
  • 14 unit tests covering valid configs, all error paths, and optional-section
    handling

Design notes

Follows the existing v1 chemistry parser pattern: std::expected<T, Errors>
return type, ErrorCode enum errors, yaml-cpp kept private to the library.
Three intentional differences from the MICM parser shape are noted for
reviewers:

  1. No Parse(YAML::Node) overload — emissions configs are always file/string
    entry points
  2. No separate CheckSchema()/Build() split — validation and construction
    are a single pass
  3. Types live in emissions::v1::types (deeper namespace) to avoid collision
    with mechanism_configuration::types

Aside for MC maintainers

While reviewing this I noticed that yaml-cpp headers currently leak into
MC's public headers in a few places (direct YAML::Node parameters on
public APIs). This isn't blocking for MIEM (MIEM only links against MUSICA,
not MC directly), but it could cause issues for downstream consumers. Flagging
here in case the maintainers want to address it in a follow-up.

Test plan

  • All 14 new emissions tests pass
  • All 19 existing chemistry tests still pass (no regressions)
  • Build tested on Derecho with -DMECH_CONFIG_USE_FMT=ON (Intel LLVM,
    no <format> header)

Adds a new emissions configuration schema under
mechanism_configuration::emissions::v1, parallel to the existing
chemistry schema. This is the MC side of the MIEM config pipeline
described in NCAR/miem#272.

New public API:
  ParseEmissions(path) / ParseEmissions(string)
  → std::expected<EmissionsConfig, Errors>

EmissionsConfig holds inventories, species maps, regridding settings,
and a list of SourceDescriptors. Musica's translator will resolve these
into miem::Source structs and drive miem::EmissionsBuilder.

Load-time validation covers all v1 invariants: kind/version dispatch,
cross-reference checks (inventory and species map names), duplicate
source names, duplicate (category, hierarchy) pairs, scaling factor
sums ≤ 1.0, and hard rejections for mode: online, vertical injection:
plume, and regridding type: scrip (none supported in v1).

Note for MC maintainers: yaml-cpp is currently linked PUBLIC in
src/CMakeLists.txt, which causes it to leak into downstream consumers.
Demoting it to PRIVATE (and moving the three public headers that include
yaml-cpp into src/internal/) would be a clean follow-up.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@smeechncar smeechncar requested a review from K20shores June 25, 2026 17:35
@codecov-commenter

codecov-commenter commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.81481% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.96%. Comparing base (6633a50) to head (42f29c2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/v1/emissions_parser.cpp 89.26% 19 Missing ⚠️
src/errors.cpp 0.00% 10 Missing ⚠️
src/validate.cpp 95.12% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
+ Coverage   88.93%   88.96%   +0.03%     
==========================================
  Files          38       39       +1     
  Lines        1627     1949     +322     
==========================================
+ Hits         1447     1734     +287     
- Misses        180      215      +35     
Files with missing lines Coverage Δ
include/mechanism_configuration/errors.hpp 57.14% <ø> (ø)
include/mechanism_configuration/mechanism.hpp 100.00% <ø> (ø)
src/detail/v1/parser.hpp 100.00% <ø> (ø)
src/v1/parser.cpp 87.04% <100.00%> (+3.20%) ⬆️
src/v1/utils.cpp 100.00% <100.00%> (ø)
src/validate.cpp 89.34% <95.12%> (+4.12%) ⬆️
src/errors.cpp 35.13% <0.00%> (-13.02%) ⬇️
src/v1/emissions_parser.cpp 89.26% <89.26%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@smeechncar

Copy link
Copy Markdown
Contributor Author

Next steps (for context)...

This PR is the first of three pieces. Once merged, the plan is:

  1. MUSICA — add a translator (musica::emissions::Translate) that calls ParseEmissions, walks the resulting EmissionsConfig, and constructs MIEM's EmissionsBuilder / Source objects. MUSICA is the only layer that sees both MC and MIEM types.
  2. MIEM integration test — an end-to-end test using a real CAMS-GLOB NetCDF file to verify the full config → translate → runtime pipeline.

Flagging so reviewers know the public types in emissions::v1::types and the two ParseEmissions overloads are the stable interface MUSICA will depend on.

@K20shores K20shores requested a review from boulderdaze June 25, 2026 20:53

@K20shores K20shores left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We want the description of emissions to be agnostic of the miem tool. I think it would be better to include a new section in the mechanism object called Emissions and put all created structs into that.

This would mean our v1.x format would have a new keyword, emissions, which essentially takes lines 5-end of examples/emissions/v1/emissions.yaml and indents it under the emissions key.

I guess I'll think about this a bit longer before adding more specific comments.

@boulderdaze boulderdaze left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would you mind updating the PR title? We include PR titles in the release notes, so we prefer them to be more descriptive and easier to read

Comment thread include/mechanism_configuration/emissions/v1/parse.hpp Outdated
@smeechncar smeechncar changed the title feat: add emissions::v1 config parser for MIEM integration Add emissions configuration schema (inventories, species maps, sources) to mechanism v1 Jun 26, 2026
Aligns with the library convention where all public headers live directly
under include/mechanism_configuration/ without a version subdirectory.
The v1 version is still expressed in the namespace and source layout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread examples/emissions/v1/emissions.yaml Outdated
Comment thread include/mechanism_configuration/emissions/v1/parse.hpp Outdated
smeechncar and others added 3 commits June 26, 2026 11:21
…er, or file

Addresses K20shores' review of #281:
- Emissions types moved into mechanism_configuration::types (types.hpp)
- EmissionsConfig added as std::optional<types::EmissionsConfig> on Mechanism
- emissions: key parsed as an optional section by the main v1 Parse() call
- Standalone ParseEmissions() API and emissions::v1 namespace removed
- data_root and shell expansion dropped for JSON parity
- Examples updated: emissions.yaml gone; full_configuration.yaml gains an
  emissions: section
- Tests routed through Parse() instead of the old standalone parser

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The public Parse() only accepted a filesystem::path, so passing a
std::string content literal silently converted to a path and failed
with FileNotFound. Dispatches to v1::Parser for version 1 documents.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ser for string tests

ResolveFileConfig only forwarded species/phases/reactions into the
combined node; the emissions: map was silently dropped so file-based
parses never saw it. Added explicit copy of the emissions key alongside
the existing version/name handling.

Reverted the public Parse(std::string) overload added in the previous
commit — it broke existing callers that passed std::string file paths
(they started routing to the content parser instead of the path
parser). String-based emission tests now use v1::Parser{} directly,
consistent with how the existing integration tests work.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@K20shores K20shores left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Getting there, a couple of more changes. We also need to be able to call Validate and have the emissions be checked. It would be nice. After adding the validation code (which probably involves expanding the semantic refernces to work with emissions) please also update test/unit/test_validate.cpp so that it does an in-code check of emissions

Comment thread examples/v1/full_configuration.yaml
Comment thread examples/v1/full_configuration.yaml Outdated
Comment thread examples/v1/full_configuration.yaml Outdated
Comment thread include/mechanism_configuration/types.hpp
Comment thread include/mechanism_configuration/types.hpp
Comment thread src/v1/parser.cpp
Comment thread src/v1/parser.cpp Outdated
… check emissions in Validate()

Addresses K20shores' second review round on #281:
- inventories/species maps changed from YAML-key-is-name maps to lists
  of objects with an explicit `name` field; Inventory/SpeciesMap structs
  gained a name field and EmissionsConfig fields moved from
  unordered_map to vector to match
- Emissions parsing moved out of src/v1/parser.cpp into its own
  src/detail/v1/emissions_parser.hpp + src/v1/emissions_parser.cpp,
  mirroring the existing CheckXSchema/ParseX split used by
  species/phases/reactions
- Parser::Build() no longer takes an emissions parameter; emissions are
  parsed and assigned after Build() in ValidateAndBuild
- Validate() now checks emissions semantics (duplicate inventory/
  species-map/source names, duplicate (category, hierarchy) pairs,
  unknown inventory/species-map references, scaling-factor sums) for
  in-code Mechanisms, not just YAML parses — extended semantics::Input
  with EmissionsRef/SpeciesMapRef/SourceRef and added the checks to
  ValidateSemantics. This also tightens behavior: a source referencing
  an unknown inventory/species-map is now always rejected, even when
  the inventories/species maps section is entirely absent (previously
  silently accepted in that case)
- full_configuration.json gained a matching emissions block (had none
  before); full_configuration.yaml and all emissions test fixtures
  updated to the new list shape
- test_validate.cpp gained 9 in-code emissions cases

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@smeechncar

smeechncar commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Re: needing Validate to check emissions — done in 51c4647. Emissions are now checked by Validate() for in-code Mechanisms too, not just YAML parses: extended semantics::Input with EmissionsRef/SpeciesMapRef/SourceRef and moved the duplicate/unknown-reference/scaling-sum checks into ValidateSemantics. Added 9 new cases to test/unit/test_validate.cpp covering this.

One deliberate tightening worth flagging: a source referencing an unknown inventory/species-map is now always rejected, even if that section is entirely absent from the config. Previously that combination silently passed. Let me know if you'd rather it stay permissive there.

@boulderdaze boulderdaze left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. I don't have any additional comments beyond the ones already mentioned

@K20shores K20shores left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems good to me. I think in the future we may add some validation to check that species are mapped to valid species defined in the mechanism, but we probably don't need to require that check yet. This looks good to me

Resolves the include-order conflict in src/v1/parser.cpp and adds the
detail/error_format.hpp include to emissions_parser.cpp, which was added
on this branch before main's ErrorLocation formatter was moved out of
the public errors.hpp header.
@smeechncar

Copy link
Copy Markdown
Contributor Author

Merged main into this branch to resolve the conflict that popped up after approval — it was trivial: both branches had added a new #include at the same spot in src/v1/parser.cpp (this branch's detail/v1/emissions_parser.hpp vs. main's detail/error_format.hpp). Kept both, alphabetized.

That merge also pulled in main's refactor moving the ErrorLocation fmt formatter out of the public errors.hpp into src/detail/error_format.hpp. src/v1/emissions_parser.cpp was added on this branch before that refactor landed, so it needed the same include added to keep building. Rebuilt and ran the full ctest suite locally (33/33 passing) before pushing. PR is mergeable now.

@smeechncar

Copy link
Copy Markdown
Contributor Author

Thank you both @boulderdaze @K20shores for reviewing this one!

@smeechncar smeechncar merged commit a8c7c0d into main Jul 2, 2026
27 of 28 checks passed
@smeechncar smeechncar deleted the feature/emissions-v1 branch July 2, 2026 23:48
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.

Add MIEM support (v1.2)

4 participants