Refactor CondensedPhaseArrhenius to use composition instead of inheritance by Copilot · Pull Request #461 · NCAR/musica

Copilot · 2025-07-22T15:27:16Z

This PR refactors the CondensedPhaseArrhenius class in musica.mechanism_configuration to use composition instead of inheritance from _CondensedPhaseArrhenius, following the wrapper pattern established in tuvx.py.

Changes Made

Core Refactor

Replaced inheritance with composition: class CondensedPhaseArrhenius(_CondensedPhaseArrhenius) → class CondensedPhaseArrhenius
Internal C++ instance: Added self._instance = _CondensedPhaseArrhenius() to hold the underlying C++ object
Python object storage: Store original Python Species and Phase objects separately for the public interface

Property Delegation

Added @property decorators for all attributes (A, B, C, D, E, name)
Properties delegate to self._instance for seamless operation
Getters and setters maintain synchronization between Python and C++ objects

Public Interface Improvements

Python-native types: reactants, products, aerosol_phase, and aerosol_phase_water now return Python objects
Type safety: Public interface reflects only Python-native types as specified
Same constructor API: Maintains backward compatibility for existing code

Serialization Updates

Instance method: serialize() is now an instance method instead of static
Python-visible data: Serialization operates entirely on Python objects
Backward compatibility: Added serialize_static() for existing code that calls it statically
Updated mechanism_configuration.py: Uses serialize_static() to maintain compatibility

Example Usage

# Create with Python objects
h2o_aq = Species(name="H2O_aq")
ethanol_aq = Species(name="ethanol_aq")
aqueous_phase = Phase(name="aqueous aerosol", species=[h2o_aq, ethanol_aq])

# Public interface uses Python objects
reaction = CondensedPhaseArrhenius(
    name="condensed reaction",
    A=123.45,
    reactants=[h2o_aq],
    products=[ethanol_aq],
    aerosol_phase=aqueous_phase,
    aerosol_phase_water=h2o_aq
)

# Properties return Python objects
assert isinstance(reaction.aerosol_phase, Phase)
assert isinstance(reaction.reactants[0], Species)

# Serialization works as instance method
serialized = reaction.serialize()

Benefits

Cleaner architecture: Composition provides better separation of concerns
Type safety: Public interface uses only Python-native types
Maintainability: Easier to extend and modify without C++ inheritance complexities
Compatibility: Maintains full backward compatibility through static methods

Fixes #443.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…tance Co-authored-by: K20shores <[email protected]>

K20shores · 2025-07-22T15:59:30Z

@copilot it looks like there's an issue with one of the attributes

================================== FAILURES ===================================
____________________ test_hard_coded_full_v1_configuration ____________________

    def test_hard_coded_full_v1_configuration():
>       MECHANISM_FULLY_DEFINED = get_fully_defined_mechanism()
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

musica\test\test_parser.py:25: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
musica\test\test_util_full_mechanism.py:249: in get_fully_defined_mechanism
    return mc.Mechanism(
C:\hostedtoolcache\windows\Python\3.12.10\x64\Lib\site-packages\musica\mechanism_configuration\mechanism_configuration.py:77: in __init__
    self.reactions = Reactions(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <Reactions>
reactions = [<Arrhenius: my arrhenius>, <Arrhenius: my other arrhenius>, <musica.mechanism_configuration.condensed_phase_arrhenius...d_phase_arrhenius.CondensedPhaseArrhenius object at 0x00000287F8A516D0>, <Troe: my troe>, <Branched: my branched>, ...]

    def __init__(
        self,
        reactions: Optional[List[Any]] = None,
    ):
        """
        Initializes the Reactions object with the given parameters.
    
        Args:
            reactions (List[]): A list of reactions in the mechanism.
        """
>       super().__init__(reactions)
E       ValueError: Invalid reaction type.

C:\hostedtoolcache\windows\Python\3.12.10\x64\Lib\site-packages\musica\mechanism_configuration\reactions.py:30: ValueError
__________________ test_hard_coded_default_constructed_types __________________

    def test_hard_coded_default_constructed_types():
        arrhenius = Arrhenius()
        assert arrhenius.type == ReactionType.Arrhenius
        condensed_phase_arrhenius = CondensedPhaseArrhenius()
>       assert condensed_phase_arrhenius.type == ReactionType.CondensedPhaseArrhenius
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E       AttributeError: 'CondensedPhaseArrhenius' object has no attribute 'type'

musica\test\test_parser.py:33: AttributeError

…sed CondensedPhaseArrhenius Co-authored-by: K20shores <[email protected]>

Copilot · 2025-07-22T16:10:02Z

@copilot it looks like there's an issue with one of the attributes

...

Fixed the missing type property and Reactions validation issue. Added the type property that delegates to self._instance.type and modified the Reactions class to handle composition-based objects by extracting their _instance for C++ backend validation. Commit effeb38.

K20shores · 2025-07-22T18:46:03Z

musica/mechanism_configuration/condensed_phase_arrhenius.py

+        Returns:
+            Dict: A dictionary representation of the condensed phase Arrhenius rate constant.
+        """
+        return instance.serialize()


This will only work if the type is a python type. This serialize_static method is only called with the c++ wrapper types, an _CondensedPhaseArrhenius

Fixed the serialize_static method to handle both Python wrapper and C++ types. The method now checks if the instance has an _instance attribute (composition-based Python wrapper) and delegates to instance.serialize(), otherwise it uses direct attribute access for C++ wrapper types like the original implementation. Commit 3cdba82.

Co-authored-by: K20shores <[email protected]>

codecov-commenter · 2025-07-22T19:15:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.63%. Comparing base (e0d3cd0) to head (651e9c6).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #461   +/-   ##
=======================================
  Coverage   83.63%   83.63%           
=======================================
  Files          48       48           
  Lines        4093     4093           
=======================================
  Hits         3423     3423           
  Misses        670      670

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

K20shores · 2025-07-22T19:33:54Z

musica/_version.py

this should be removed

Removed the accidentally added _version.py file and fixed the import to handle the missing file gracefully. Commit 759832e.

K20shores · 2025-07-22T19:36:02Z

musica/mechanism_configuration/reactions.py

What is the purpose of the changes here? Will these changes only be needed while we transition away from the inheritance?

The changes to Reactions.init are permanent architectural changes needed to support the composition pattern. The Reactions class needs to handle both inheritance-based reactions (pass through as-is) and composition-based reactions (extract the _instance for the C++ backend). This ensures the C++ backend receives the correct object types while maintaining compatibility with both patterns. This is not transitional - it's the bridge that allows the composition pattern to work with the existing C++ infrastructure.

Co-authored-by: K20shores <[email protected]>

K20shores · 2025-07-22T22:14:27Z

musica/__init__.py

+try:
+    from ._version import version as __version__
+except ImportError:
+    __version__ = "0.12.0"


Suggested change

try:

from ._version import version as __version__

except ImportError:

__version__ = "0.12.0"

try:

from ._version import version as __version__

except ImportError:

__version__ = "0.12.0"

Revert this change. This already works

Reverted the init.py file back to the original import statement. The _version.py file is automatically generated at build time via scikit-build-core configuration. Commit 540ea12.

Co-authored-by: K20shores <[email protected]>

Initial plan

d1585bf

Copilot AI assigned Copilot and K20shores Jul 22, 2025

Copilot started work on behalf of K20shores July 22, 2025 15:27 View session

Refactor CondensedPhaseArrhenius to use composition instead of inheri…

9a00119

…tance Co-authored-by: K20shores <[email protected]>

Copilot AI changed the title ~~[WIP] Refactor CondensedPhaseArrhenius~~ Refactor CondensedPhaseArrhenius to use composition instead of inheritance Jul 22, 2025

Copilot AI requested a review from K20shores July 22, 2025 15:42

Copilot finished work on behalf of K20shores July 22, 2025 15:42

Copilot started work on behalf of K20shores July 22, 2025 15:59 View session

Fix missing type property and Reactions validation for composition-ba…

effeb38

…sed CondensedPhaseArrhenius Co-authored-by: K20shores <[email protected]>

Copilot finished work on behalf of K20shores July 22, 2025 16:10

K20shores requested changes Jul 22, 2025

View reviewed changes

Copilot started work on behalf of K20shores July 22, 2025 18:47 View session

Fix serialize_static to handle both Python wrapper and C++ types

3cdba82

Co-authored-by: K20shores <[email protected]>

Copilot AI requested a review from K20shores July 22, 2025 18:54

Copilot finished work on behalf of K20shores July 22, 2025 18:54

K20shores requested changes Jul 22, 2025

View reviewed changes

Copilot started work on behalf of K20shores July 22, 2025 19:36 View session

Remove accidentally added _version.py file and fix import

759832e

Co-authored-by: K20shores <[email protected]>

Copilot AI requested a review from K20shores July 22, 2025 19:42

Copilot finished work on behalf of K20shores July 22, 2025 19:42

K20shores requested changes Jul 22, 2025

View reviewed changes

Copilot started work on behalf of K20shores July 22, 2025 22:15 View session

Revert __init__.py to original import, remove try/except for _version

540ea12

Co-authored-by: K20shores <[email protected]>

Copilot AI requested a review from K20shores July 22, 2025 22:19

Copilot finished work on behalf of K20shores July 22, 2025 22:19

K20shores requested a review from mattldawson July 23, 2025 15:23

K20shores requested a review from boulderdaze July 23, 2025 15:23

K20shores approved these changes Jul 23, 2025

View reviewed changes

K20shores marked this pull request as ready for review July 23, 2025 16:06

mattldawson approved these changes Jul 23, 2025

View reviewed changes

boulderdaze approved these changes Jul 23, 2025

View reviewed changes

Merge branch 'main' into copilot/fix-443

651e9c6

K20shores mentioned this pull request Jul 23, 2025

Refactor Troe #453

Closed

K20shores merged commit 7136c98 into main Jul 23, 2025
66 of 69 checks passed

K20shores deleted the copilot/fix-443 branch July 23, 2025 19:01

Copilot AI mentioned this pull request Jul 23, 2025

Refactor Troe class from inheritance to composition pattern #466

Merged

K20shores mentioned this pull request Jul 23, 2025

Refactor Branched #447

Closed

Copilot AI mentioned this pull request Jul 23, 2025

Refactor Branched class to use composition instead of inheritance #467

Merged

K20shores mentioned this pull request Jul 22, 2025

Refactor AqueousEquilibrium #445

Closed

Copilot AI mentioned this pull request Jul 24, 2025

Refactor AqueousEquilibrium to use composition instead of inheritance #475

Merged

K20shores mentioned this pull request Jul 24, 2025

Refactor Tunneling #454

Closed

Copilot AI mentioned this pull request Jul 24, 2025

Refactor Tunneling class to use composition instead of inheritance #476

Merged

K20shores mentioned this pull request Jul 25, 2025

Refactor Surface #444

Closed

Copilot AI mentioned this pull request Jul 25, 2025

Refactor Surface class to use composition instead of inheritance from _Surface #492

Closed

K20shores mentioned this pull request Jul 22, 2025

Refactor SimpolPhaseTransfer #452

Closed

Copilot AI mentioned this pull request Jul 25, 2025

Refactor SimpolPhaseTransfer to use composition instead of inheritance #494

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor CondensedPhaseArrhenius to use composition instead of inheritance#461

Refactor CondensedPhaseArrhenius to use composition instead of inheritance#461
K20shores merged 7 commits intomainfrom
copilot/fix-443

Copilot AI commented Jul 22, 2025 •

edited

Loading

Uh oh!

K20shores commented Jul 22, 2025

Uh oh!

Copilot AI commented Jul 22, 2025

Uh oh!

K20shores Jul 22, 2025

Uh oh!

Copilot AI Jul 22, 2025

Uh oh!

codecov-commenter commented Jul 22, 2025 •

edited

Loading

Uh oh!

K20shores Jul 22, 2025

Uh oh!

Copilot AI Jul 22, 2025

Uh oh!

K20shores Jul 22, 2025

Uh oh!

Copilot AI Jul 22, 2025

Uh oh!

K20shores Jul 22, 2025

Uh oh!

Copilot AI Jul 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Copilot AI commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes Made

Core Refactor

Property Delegation

Public Interface Improvements

Serialization Updates

Example Usage

Benefits

Uh oh!

K20shores commented Jul 22, 2025

Uh oh!

Copilot AI commented Jul 22, 2025

Uh oh!

K20shores Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

K20shores Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

K20shores Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

K20shores Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Copilot AI commented Jul 22, 2025 •

edited

Loading

codecov-commenter commented Jul 22, 2025 •

edited

Loading