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

Add Levenberg-Marquardt model calibration #31

Closed richterjakob closed 1 year ago

mrp089 commented 1 year ago

Thank you so much, @richterjakob! I will have a look with @menon-karthik. This pull request closes issue #24.

mrp089 commented 1 year ago

I added the script cali.py that does the following to all test cases:

This works nicely in a couple of test cases, but I noticed two problems:

  1. Many elements are not identifiable from how our tests are set up (like estimating C and L in steady flow).
  2. I couldn't set up a calibration in a geometry with more than a single branch. The calibrator is looking for a result that I don't know how to provide: [DEBUG MESSAGE] Reading observations for variable flow_0:J0

To address problem 1, we could add a small 0D model from the VMR as a more "real-life" calibration case. @richterjakob, can you have a look at problem 2? You can use cali.py to reproduce with test cases steadyFlow_bifurcationR_R1, steadyFlow_bifurcationR_R2, steadyFlow_blood_vessel_junction.

menon-karthik commented 1 year ago

Update: I think I got the interface and coupling to svSolver to work now. I have also created a simple C++ test case for the interface while doing this, which should be added to the github actions (#30 ). Still wondering what would be the best way to add a test case specifically for the svSolver (and eventually svFSIPlus) coupling.

One issue I'm having is running the test cases when I build the code using CMake instead of pip (on Sherlock). In this case, pytest cannot find the svzerodplus package that is required to run tests/test_integration_cpp.py. Is there a way to have this work when using the CMake installation?

I will push my changes in the next few days!

richterjakob commented 1 year ago

One issue I'm having is running the test cases when I build the code using CMake instead of pip (on Sherlock). In this case, pytest cannot find the svzerodplus package that is required to run tests/test_integration_cpp.py. Is there a way to have this work when using the CMake installation?

Unfortunately the python interface (which pytest is using) only works when installing svZeroDPlus via pip. But the pip installation should work on brocken.

mrp089 commented 1 year ago

Thanks for adding the test, @richterjakob! I'll move the cali.py script to a repository for the upcoming paper.

richterjakob commented 1 year ago

Thanks for adding the test, @richterjakob! I'll move the cali.py script to a repository for the upcoming paper.

Haha, you were faster than I could type that the only thing left is to decide whether keeping cali.py in this repository is the right choice 😄

richterjakob commented 1 year ago

But otherwise I think we are ready to merge, right?

menon-karthik commented 1 year ago

One issue I'm having is running the test cases when I build the code using CMake instead of pip (on Sherlock). In this case, pytest cannot find the svzerodplus package that is required to run tests/test_integration_cpp.py. Is there a way to have this work when using the CMake installation?

Unfortunately the python interface (which pytest is using) only works when installing svZeroDPlus via pip. But the pip installation should work on brocken.

In that case, one last thing I need to do is test the pip installation and pytest on Sherlock. If that doesn't work, it's not huge deal because people pushing from Sherlock can still use Github actions in their own branches to run the tests. But I will add a note in the documentation if that is the case. I should have this done this morning.

If there's nothing else to add (apart from moving the cali.py script), I can merge after I test that. Does that sound good @richterjakob and @mrp089 ?

richterjakob commented 1 year ago

One issue I'm having is running the test cases when I build the code using CMake instead of pip (on Sherlock). In this case, pytest cannot find the svzerodplus package that is required to run tests/test_integration_cpp.py. Is there a way to have this work when using the CMake installation?

Unfortunately the python interface (which pytest is using) only works when installing svZeroDPlus via pip. But the pip installation should work on brocken.

In that case, one last thing I need to do is test the pip installation and pytest on Sherlock. If that doesn't work, it's not huge deal because people pushing from Sherlock can still use Github actions in their own branches to run the tests. But I will add a note in the documentation if that is the case. I should have this done this morning.

If there's nothing else to add (apart from moving the cali.py script), I can merge after I test that. Does that sound good @richterjakob and @mrp089 ?

I tested the build on Sherlock already and it worked without problems.

If that's okay, I would like to do the honour of merging this 😁