Skip to content

Adding a variation of KorakianitisMixedModel with passive component #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 6, 2025

Conversation

LevanBokeria
Copy link
Collaborator

This pull request incorporates a new model instantiated by @MaxBalmus which is a variation of KorakianitisMixedModel. From Max:

A new model proposed as an alternative to KorakianitisMixedModel called KorakianitisMixedModel_PP (preserved pressure). It uses a slightly modified chamber model HC_mixed_elastance_pp, which modifies the total pressure from the form:
p(t,v) = [1-a(t)] * p_pas(v) + a(t) * p_act(v)
to:
p(t,v) = p_pas(v) + a(t) * p_act(v)
This new format prevents the pressure during contraction from being lower than the passive equivalent.

The PR also includes tests for this new model.

@LevanBokeria LevanBokeria self-assigned this Jun 5, 2025
@LevanBokeria LevanBokeria requested a review from Copilot June 5, 2025 14:11
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@LevanBokeria LevanBokeria requested review from MaxBalmus and Copilot and removed request for Copilot June 5, 2025 14:11
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new model variant, KorakianitisMixedModelPP, which implements a modified passive pressure formulation along with its corresponding tests and expected outputs. Key changes include:

  • Adding a new model in src/ModularCirc/Models/KorakianitisMixedModelPP.py that leverages a modified HC_mixed_elastance_pp component.
  • Including a new HC_mixed_elastance_pp component in src/ModularCirc/Components/HC_mixed_elastance_pp.py to compute total pressure using the updated formulation.
  • Updating tests, expected output files, and notebooks to support and validate the new model variant.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_KorakianitisMixedModelPP.py Added comprehensive tests for the new model variant and its solver configuration.
tests/expected_outputs/KorakianitisMixedModelPP_expected_output.json Contains expected output values for various solver step sizes.
src/ModularCirc/Models/KorakianitisMixedModelPP.py Implements the new model variant with modified initialization and component connections.
src/ModularCirc/Components/HC_mixed_elastance_pp.py Provides the modified component that computes total pressure per the new design.
src/ModularCirc/Components/init.py Updated to import and expose the new HC_mixed_elastance_pp component.
sandbox/notebooks/generate_KMM_KGO.ipynb & generate_KMMPP_KGO.ipynb Notebooks updated to generate known good outputs for the new model variant.
Comments suppressed due to low confidence (1)

src/ModularCirc/Models/KorakianitisMixedModelPP.py:27

  • The attribute self.name does not reflect that this is the PP variation; consider renaming it to 'KorakianitisMixedModelPP' for consistency.
self.name = 'KorakianitisModel'

Copy link
Collaborator

@MaxBalmus MaxBalmus left a comment

Choose a reason for hiding this comment

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

All good as far as I can see.

@LevanBokeria LevanBokeria merged commit 9841440 into main Jun 6, 2025
4 checks passed
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