Merged
Conversation
Contributor
|
📄 Documentation for this branch is available at: https://ncar.github.io/musica/branch/update_version/ |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #615 +/- ##
==========================================
+ Coverage 78.11% 78.25% +0.13%
==========================================
Files 54 54
Lines 6827 6861 +34
==========================================
+ Hits 5333 5369 +36
+ Misses 1494 1492 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* putting in reaction component, arrhenius, ancillary * branched * emission * first order loss * photolysis * ternary chemical * troe * user defined * tunneling, reations * removing reactions that will no longer be in v1, mechanism parsing slight refactor * v1 stuff works, still have to correct surface reactions * phase, phase species, ignoring surface reactions * updating configurations * correcting test * adding required properties for surface reactions * Including taylor series, removing others The taylor series reaction included a vector<int>. We already had a type for that in musica.cpp, but it was bound to the _core module. The taylor coefficients were in a different pybind11 module and were only causing an issue on windows. Pybind11 docs say that when you make an STL type opaque and add bindings to it, this must be done the same for all modules. * pulling in corrected test configurations * syncing some more changes * addressing copilot pr comments * addressing PR comment
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates the version to 0.13.0 and introduces significant architectural changes to support MICM v1 configuration format. The primary changes include migrating from Species to PhaseSpecies for phase-specific properties like diffusion coefficients, adding support for Taylor series reactions, and removing deprecated reaction types.
Key changes:
- Updates version to 0.13.0 and upgrades dependencies (MICM, Mech Config, TUV-x)
- Introduces
PhaseSpeciesconcept to handle diffusion coefficients at phase level instead of species level - Adds Taylor series reaction support and removes condensed phase reactions
Reviewed Changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/unit/micm/parser.cpp | Updates test assertions to use phase_species_ instead of species_ |
| src/micm/v1_parse.cpp | Major refactoring to support PhaseSpecies and Taylor series reactions |
| src/micm/v0_parse.cpp | Backward compatibility updates for v0 to v1 migration |
| src/micm/convert_v0_to_v1.cpp | Updates conversion logic from v0 to v1 format |
| pyproject.toml | Updates pybind11 version requirement to 3.0.0 |
| musica/test/unit/test_*.py | Test updates for simplified mechanism configuration |
| musica/mechanism_configuration/*.py | Complete refactor of Python bindings architecture |
| musica/mechanism_configuration.cpp | C++ binding updates for new reaction types |
| include/musica/micm/micm.hpp | Updates for PhaseSpecies support |
| configs/v1/**/*.{yaml,json} | Configuration file updates for v1 format |
| cmake/dependencies.cmake | Dependency version updates |
| CMakeLists.txt | Version bump to 0.13.0 |
Comments suppressed due to low confidence (2)
musica/test/unit/test_util_full_mechanism.py:252
- Same issue as Comment 1 - this assertion is incomplete and should include the coefficient property to match the expected reaction component structure.
assert reactions[1].type == mc.ReactionType.Arrhenius
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
24 tasks
K20shores
approved these changes
Oct 1, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Waiting for MICM PR to get merged
This PR:
micm::PhaseSpeciesin the v1 configuration, while maintaining backward compatibility with v0, where it is defined inmicm::Species