Skip to content

Allow photolysis reactions with no reactants - #312

Merged
K20shores merged 1 commit into
mainfrom
v0-photolysis-keep-no-reactants
Jul 26, 2026
Merged

Allow photolysis reactions with no reactants#312
K20shores merged 1 commit into
mainfrom
v0-photolysis-keep-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; in the CB05 config, 14 of 39 photolysis reactions are reactant-less, so they (and their irr accumulators) vanished. A converted config then fails to run: the conditions CSV supplies rates for the dropped reactions (PHOTO.EMIS_NO not found).

Fix

Make Photolysis::reactants a vector holding at most one component, and keep reactant-less reactions instead of dropping them. The reaction stays a PHOTOLYSIS (keeping its PHOTO.<name> rate-parameter label), so downstream conditions that reference it by that label keep working with no remapping.

  • types::Photolysis::reactants: ReactionComponentstd::vector<ReactionComponent>
  • v0 parser keeps the reaction with its (possibly empty) reactants
  • v1 parser parses reactants as a list; the schema already allowed 0-or-1 (it only errors on >1), and an empty list validates
  • validate.cpp and the v0/v1 photolysis tests updated (a reactant-less v0 reaction is asserted to stay a photolysis with an empty reactants list)

All 34 tests pass.

Addresses NCAR/musica#986. Pairs with a musica change to handle the now list-valued photolysis reactants in the Python layer (getter/setter/serialize), plus a mechanism_configuration tag bump; they must be released together.

Supersedes #311 (which routed these to Emission; keeping them as photolysis avoids changing the rate-parameter label, so no converter remap is needed).

🤖 Generated with Claude Code

Make Photolysis.reactants a vector (at most one element) so a reactant-less
photolysis is kept instead of dropped. Music Box Interactive encodes
emissions as reactant-less photolysis (to carry an irr product); the v0
parser previously discarded them. The reaction stays a PHOTOLYSIS (keeping
its PHOTO. rate-parameter label), so downstream configs that reference it
by that label continue to work.

- types::Photolysis::reactants: ReactionComponent -> vector (0 or 1)
- v0 parser: keep the reaction with its (possibly empty) reactants
- v1 parser: parse reactants as a list (schema already allowed 0-or-1)
- validate + tests updated; v1 accepts an empty reactants list

Addresses NCAR/musica#986. Pairs with a musica update to handle the
list-valued photolysis reactants in the Python layer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@K20shores
K20shores requested a review from boulderdaze July 26, 2026 15:49
@K20shores
K20shores merged commit fa06d8c into main Jul 26, 2026
27 of 28 checks passed
@K20shores
K20shores deleted the v0-photolysis-keep-no-reactants branch July 26, 2026 15:50
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.39%. Comparing base (9ca3910) to head (95a828f).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/validate.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #312      +/-   ##
==========================================
+ Coverage   88.08%   88.39%   +0.31%     
==========================================
  Files          44       44              
  Lines        2442     2655     +213     
==========================================
+ Hits         2151     2347     +196     
- Misses        291      308      +17     
Files with missing lines Coverage Δ
src/v0/photolysis_parser.cpp 100.00% <100.00%> (ø)
src/v1/reactions/photolysis.cpp 93.18% <100.00%> (ø)
src/validate.cpp 87.50% <0.00%> (-3.18%) ⬇️

... and 2 files with indirect coverage changes

🚀 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.

2 participants