Skip to content

Keep reactant-less v0 photolysis as an emission - #311

Closed
K20shores wants to merge 1 commit into
mainfrom
v0-photolysis-emission-no-reactants
Closed

Keep reactant-less v0 photolysis as an emission#311
K20shores wants to merge 1 commit into
mainfrom
v0-photolysis-emission-no-reactants

Conversation

@K20shores

Copy link
Copy Markdown
Collaborator

Problem

The v0 photolysis parser silently drops PHOTOLYSIS reactions whose reactants is empty ({}). Music Box Interactive encodes emissions as reactant-less photolysis so they can carry an irr__ product, e.g.:

{ "type": "PHOTOLYSIS", "__music_box_type": "EMISSION", "MUSICA name": "EMIS_NO",
  "reactants": {}, "products": {"NO": {"yield": 1}, "irr__...": {"yield": 1}} }

In the CB05 MBI config, 14 of 39 photolysis reactions are reactant-less; parse() returned only the 25 with reactants, so those emissions (and their irr accumulators) vanished. Downstream this breaks a music_box run: the conditions CSV still supplies their rates → User-defined rate parameter PHOTO.EMIS_NO not found in the mechanism.

Fix

A reactant-less photolysis is a pure production term, so parse it into the Emission list (which has no reactant) instead of dropping it. This keeps the reaction and its irr__ product. Reactions with a reactant are unchanged. Adds a v0 photolysis test case (a reactant-less reaction is now asserted to appear as an emission). All 12 v0 tests pass.

Addresses NCAR/musica#986.

Follow-ups for the full pipeline

These reactions become emissions, so their MICM rate-parameter label changes from PHOTO.<name> to EMIS.<name>. To run a converted MBI config (e.g. CB05) end-to-end, two more pieces are needed:

  1. a musica release picking up this mechanism_configuration change;
  2. the music_box config converter remapping the PHOTO.<name> rate-parameter CSV columns to EMIS.<name> for reactions that are emissions.

(An alternative design — making Photolysis.reactants a vector to allow zero reactants while keeping the PHOTO. label — was rejected here because it changes the Photolysis type and would break musica's bindings, requiring an atomic cross-repo release. Routing to Emission is self-contained.)

🤖 Generated with Claude Code

The v0 photolysis parser silently dropped PHOTOLYSIS reactions with no
reactants. Music Box Interactive encodes emissions this way (as a
photolysis so they can carry an irr product), so those reactions -- and
their irr accumulators -- were lost. A reactant-less photolysis is a pure
production term, so parse it into the Emission list (which has no reactant)
instead of dropping it. Adds a v0 photolysis test case.

Relates to NCAR/musica#986.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.42%. Comparing base (9ca3910) to head (a9f1d47).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #311      +/-   ##
==========================================
+ Coverage   88.08%   88.42%   +0.34%     
==========================================
  Files          44       44              
  Lines        2442     2661     +219     
==========================================
+ Hits         2151     2353     +202     
- Misses        291      308      +17     
Files with missing lines Coverage Δ
src/v0/photolysis_parser.cpp 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

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

@K20shores

Copy link
Copy Markdown
Collaborator Author

Superseded by #312: keep reactant-less reactions as photolysis (vector reactants) instead of routing to emission, so the PHOTO. rate-parameter label is preserved and no converter remap is needed.

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.

2 participants