Skip to content
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

Add Levenberg-Marquardt model calibration #31

Merged
merged 87 commits into from
Sep 1, 2023

Conversation

richterjakob
Copy link
Member

No description provided.

@menon-karthik
Copy link
Member

menon-karthik commented Aug 29, 2023

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!

@menon-karthik menon-karthik added the enhancement New feature or request label Aug 30, 2023
Copy link
Member

@menon-karthik menon-karthik left a comment

Choose a reason for hiding this comment

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

Looks great to me for the most part! The only problem is pytest not working when using the CMake build on Sherlock.

.github/workflows/test.yml Show resolved Hide resolved
@richterjakob
Copy link
Member Author

richterjakob commented Sep 1, 2023

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
Copy link
Member

mrp089 commented Sep 1, 2023

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

@richterjakob
Copy link
Member Author

richterjakob commented Sep 1, 2023

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
Copy link
Member Author

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

@menon-karthik
Copy link
Member

menon-karthik commented Sep 1, 2023

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
Copy link
Member Author

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 😁

@richterjakob richterjakob merged commit 0ba4490 into SimVascular:master Sep 1, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include least squares parameter optimization Installing/Setting Up C++ version using pip.
4 participants