-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
… its the same one as before
…tion-pressure 66 improved contraction pressure
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this 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'
There was a problem hiding this 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.
This pull request incorporates a new model instantiated by @MaxBalmus which is a variation of KorakianitisMixedModel. From Max:
The PR also includes tests for this new model.