search

SimVascular / svZeroDSolver

A C++ lumped-parameter solver for blood flow and pressure in hemodynamic networks
https://simvascular.github.io/documentation/rom_simulation.html#0d-solver
Other
7 stars 18 forks source link

Added option to specify cardiac period #127

Open rjrios915 opened 2 months ago

rjrios915 commented 2 months ago

Addresses #126

Added option to specify cardiac period under simulation parameters

Current situation

Link any open issues or PRs related to this PR. Please ensure that all non-trivial PRs are first tracked and discussed in an existing GitHub issue or discussion.

Release Notes

Add a bullet point list summary of the feature and possible migration guides if this is a breaking change so this section can be added to the release notes.

Documentation

Please ensure that you properly document any additions.

Testing

Please ensure that the PR meets the testing requirements set by GitHub Actions.

In addition, please ensure all modified and new code is covered by tests.

Code of Conduct & Contributing Guidelines

menon-karthik commented 2 months ago

@rjrios915 @JonathanPham Please make sure the tests are passing!

Also, just wondering if it might be better to specify the cardiac cycle period within the relevant block that is driving the cardiac cycle (like the cardiac chamber)? For example, the currently implemented ClosedLoopHeartPulmonary block has this implemented within the block. It might make more sense to do this because the cycle period is not a parameter of the entire simulation really, it is only a parameter of the block that is driving the heart beat.

menon-karthik commented 1 month ago

@rjrios915 @JonathanPham Not sure why the tests are still failing but can you rebase this branch with the master so it it using the most recent version please?

rjrios915 commented 1 month ago

Hello! All test cases except the data visualization should now be passing. As for where the cardiac period is defined, I think it could be useful to do so globally. The solver already utilizes a global period to calculate step size, number of cycles, and the time per cycle when outputting a single cardiac cycle even when there is no heart block, so I was thinking adding it to simulation parameters would make it easy for users to change the value if needed.

rjrios915 commented 2 weeks ago

Hello! We ran into the issue where the initialization of cardiac period to 1 within the model and simulation parameters separately cause an error when one would automatically be 1 but the other some valid non 1 value. To resolve this, both the model and parameters initialize the cardiac period to -1 unless specified elsewhere. If both values are -1, then solver.h sets the value to 1 (like what was happening in Model::Finalize). If both values are larger than 0, there is a check to make sure the values are consistent. If one is -1 and the other is greater than 0, then that value is set as the cardiac cycle.

The other idea we had was to pass the parameters to the model constructor so this process could happen like originally when the model is initialized, but I was unsure of how to go about this.

Let me know your thoughts! All the tests should be passing.

ncdorn commented 1 week ago

@rjrios915 ensure that documentation for cardiac period has been added