-
Notifications
You must be signed in to change notification settings - Fork 20
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
Simulate an open-loop 0D model until it converges #109
Conversation
…bsolute error in mean flow and pressure between last two cardiac cycles is less than user-specified value; added coronary test case to test this functionality
…erent for models with RCR boundary conditions, where simulation instead runs for an estimated number of cardiac cycles based on the RCR time constant (estimate from Pfaller 2021 paper)
@menon-karthik or @mrp089, can one of you guys review the code and merge the pull request? Thanks! |
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.
This looks really great @JonathanPham, thanks for doing this!
I have one minor comment (apart from the one about the print message) - would it be possible to for you to check what the computational cost of checking the convergence is? Maybe take the test cases you added, run them ~100 times with the convergence check on and ~100 times with the convergence check off (but the same number of cardiac cycles). Just wondering what the impact of this check is on the performance, and if it is measurable we can add a note to the documentation saying so. What do you think?
src/solve/Solver.cpp
Outdated
converged = check_vessel_cap_convergence(states_last_two_cycles, | ||
vessel_caps_dof_indices); | ||
} | ||
DEBUG_MSG("Ran simulation for " << extra_num_cycles |
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.
I think we should print this message even in non-debug builds so the user has this information when it occurs. Similar to the messages below showing the percentage error in flow/pressure.
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.
That's a good idea. I can check on the computational cost and report back. I can also print the above message as well.
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.
In the following tests, I set use_cycle_to_cycle_error
to be either true
or false
. In both cases, number_of_cardiac_cycles
is set to be the same value. The times reported below correspond to the total time needed to run each case 100 times.
Note that for both tests, I set number_of_cardiac_cycles
to be the total number of cardiac cycles found when use_cycle_to_cycle_error
is true
.
- Test
pulsatileFlow_R_coronary_cycle_error.json
:true
takes 12.9 seconds andfalse
takes 1.7 seconds --> 7.5x slowdown when checking cycle-to-cycle errors - Test
pulsatileFlow_bifurcationR_RCR_cycle_error.json
:true
takes 0.093 seconds andfalse
takes 0.083 seconds --> 1.12x slowdown to compute new number of cardiac cycles
The above timing results suggest that with the current code, when we check for cycle-to-cycle errors, the computational cost increases almost an order of magnitude for models without RCR boundary conditions. This is quite expensive. However, I have an idea of how to update the code to make this faster. Will work on adding these updates next.
Script used for timings:
test_times.txt
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.
Just reran the above timing tests with the newest commit 19d7595 and the timings are pretty much equivalent now (meaning there is no significant slowdown from checking the cycle-to-cycle errors).
@menon-karthik I think we are good to merge now.
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.
Great, glad you checked this!
Addresses issue #17.
These updates to give users the option to simulate an open-loop 0D model until it converges.
In the 0D solver input file, users must specify the number of cardiac cycles,$n$ , they want to run (this parameter is $\epsilon_{target}$ (parameter,
number_of_cardiac_cycles
in the 0D input file). They will also have the option (via parameter,use_cycle_to_cycle_error
in the 0D input file) to specify a target cycle-to-cycle error,sim_cycle_to_cycle_percent_error
, in the 0D input file).If only$n$ is specified, then we will just run the 0D simulation for $n$ cycles.
If$\epsilon_{target}$ is also prescribed, then we take separate steps depending on whether or not the 0D model includes a RCR boundary condition.
We also created a new test case for each case above.
pulsatileFlow_R_coronary.json
, where we output results at all time steps. Then, we simulate the 0D model, compute the mean flow and pressure for the last two cardiac cycles simulated, and confirm thatsteadyFlow_bifurcationR_R2.json
, where we output results at all time steps and the two RCR boundary conditions have different time constants. Also, use a periodic inflow instead of a steady inflow. We computenumber_of_cardiac_cycles
to be a value that is different fromnumber_of_cardiac_cycles
.