Skip to content

Refactor AqueousEquilibrium to use composition instead of inheritance#475

Merged
K20shores merged 4 commits intomainfrom
copilot/fix-445
Jul 25, 2025
Merged

Refactor AqueousEquilibrium to use composition instead of inheritance#475
K20shores merged 4 commits intomainfrom
copilot/fix-445

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jul 24, 2025

This PR refactors the AqueousEquilibrium class in musica.mechanism_configuration to use composition instead of inheritance from _AqueousEquilibrium, following the established pattern used in other reaction classes like Arrhenius, Troe, and Branched.

Changes Made

1. Composition Pattern Implementation

  • Removed inheritance: Changed from class AqueousEquilibrium(_AqueousEquilibrium): to class AqueousEquilibrium:
  • Added composition: The internal C++ instance is now held in self._instance = _AqueousEquilibrium()
  • Property delegation: All attributes now use @property decorators that delegate to self._instance

2. Python/C++ Object Conversion

  • Reactants/Products: Implemented seamless conversion between Python Species objects and C++ _ReactionComponent objects
  • Type safety: Proper handling of coefficient tuples (float, Species) in reactants and products
  • Phase handling: Supports both Phase objects and string names for aerosol_phase and aerosol_phase_water

3. Serialize Method Refactoring

  • Instance method: Converted serialize() from static method to instance method operating on Python-visible data
  • Backward compatibility: Added serialize_static() method for compatibility with C++ _AqueousEquilibrium objects
  • Updated integration: Fixed mechanism_configuration.py to handle both Python and C++ objects correctly

4. Complete Property Coverage

All properties now properly delegate to self._instance:

  • name, aerosol_phase, aerosol_phase_water
  • reactants, products (with Python ↔ C++ conversion)
  • A, C, k_reverse, other_properties
  • type (returns ReactionType.AqueousEquilibrium)

Example Usage

import musica.mechanism_configuration as mc

# Create species and phase objects
water = mc.Species(name="H2O_aq")
aerosol = mc.Phase(name="aqueous_aerosol")
reactant = mc.Species(name="A")
product = mc.Species(name="B")

# Create aqueous equilibrium with composition pattern
aq_eq = mc.AqueousEquilibrium(
    name="my_equilibrium",
    aerosol_phase=aerosol,           # Accepts Phase object
    aerosol_phase_water=water,       # Accepts Species object  
    reactants=[(2.0, reactant)],     # Coefficient + Species tuple
    products=[product],              # Species object
    A=1.14e-2,
    C=2300.0,
    k_reverse=0.32
)

# All properties return Python objects
print(aq_eq.reactants)  # Returns List[Union[Species, Tuple[float, Species]]]
print(aq_eq.serialize())  # Instance method, operates on Python data

Pattern Consistency

This refactoring exactly matches the composition pattern established in:

  • arrhenius.py
  • troe.py
  • branched.py
  • condensed_phase_arrhenius.py

Benefits

  • Clean separation: Public interface exposes only Python-native types
  • Type safety: Proper conversion between Python and C++ objects
  • Maintainability: Consistent pattern across all reaction classes
  • Backward compatibility: Existing code continues to work with both object types

Fixes #445.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI changed the title [WIP] Refactor AqueousEquilibrium Refactor AqueousEquilibrium to use composition instead of inheritance Jul 24, 2025
Copilot AI requested a review from K20shores July 24, 2025 16:36
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.85%. Comparing base (f9c9017) to head (8280e80).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #475      +/-   ##
==========================================
+ Coverage   83.63%   84.85%   +1.22%     
==========================================
  Files          48       49       +1     
  Lines        4093     4815     +722     
==========================================
+ Hits         3423     4086     +663     
- Misses        670      729      +59     

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

Returns:
Dict: A dictionary representation of the AqueousEquilibrium object.
"""
serialize_dict = {
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.

create a temporary object and call serialize so that we don't duplicated the dictionary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! I've added a _create_serialize_dict() helper method to eliminate the code duplication between serialize() and serialize_static(). The serialize_static() method now creates a temporary object and calls the helper method, following the exact same pattern used in Arrhenius, Troe, and other reaction classes. Commit: 8280e80

Copilot AI requested a review from K20shores July 24, 2025 18:41
@K20shores K20shores marked this pull request as ready for review July 24, 2025 21:30
@K20shores K20shores merged commit 492ee5f into main Jul 25, 2025
70 checks passed
@K20shores K20shores deleted the copilot/fix-445 branch July 25, 2025 12:30
@K20shores K20shores mentioned this pull request Jul 25, 2025
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.

Refactor AqueousEquilibrium

5 participants