Skip to content

Conversation

@hanzhao2020
Copy link
Contributor

Adding unfitted resistive immersed surface (RIS) method functions for heart valve CFD simulation.

Current situation

Resolves #222 for the unfitted RIS method to simulate hemodynamics with heart valves.

Release Notes

Added unfitted RIS functions in uris.cpp and updated related files. Valve open and close motions are prescribed in the simulation and are given as input data.

Documentation

The code is documented with comments. Detailed documentation for the RIS method will be added to the svMultiPhysics website.

Testing

A test case is added for a cylinder geometry with two immersed valves.

Code of Conduct & Contributing Guidelines

@hanzhao2020 hanzhao2020 requested a review from ktbolt August 12, 2025 15:55
@ktbolt
Copy link
Collaborator

ktbolt commented Aug 13, 2025

@hanzhao2020 Your code does not compile.

@hanzhao2020
Copy link
Contributor Author

@ktbolt Thanks for the heads up. There was a duplicate definition of bcast for an int array. This issue has been fixed, and the code can now compile. But the CANN model test cases are now failing in parallel. I will look into that.

Copy link
Collaborator

@ktbolt ktbolt left a comment

Choose a reason for hiding this comment

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

@hanzhao2020 Note that this has already been fixed in the main branch and was a problem because your branch is out of date.

@hanzhao2020
Copy link
Contributor Author

@ktbolt Thanks! This issue was fixed. The current code passed the Ubuntu test, but failed to build for macos test. The code built successfully on my macos laptop. Have you seen this build error before?

@ktbolt
Copy link
Collaborator

ktbolt commented Aug 19, 2025

@hanzhao2020 I've not seen this problem.

@aabrown100-git @dcodoni Have you see this before ? Seems like the CMake variable CURRENT_OSX_VERSION is not set correctly.

@dcodoni
Copy link
Contributor

dcodoni commented Aug 20, 2025

@ktbolt I have never run into this kind of error. The OS github is loading is the same of the previous times and it has always worked fine. @hanzhao2020 did you merge your branch with the main one? I am wondering if all your cmake files are correct.

@ktbolt I saw in the SimVascularSystemSetup.cmake file:
elseif(APPLE)
#uname -p does not work correctly on OS X, we are going to assume its x64
SET(ARCH "x64")
set(IS64 TRUE)

but the loaded os in github is arm64. It has never created a problem before, but it is the only source of problem I could see. @hanzhao2020 could you try to run the test again?

@hanzhao2020
Copy link
Contributor Author

@dcodoni Thanks for looking into this. I run the test again, but got the same build error.

@aabrown100-git
Copy link
Collaborator

Something must have changed on GitHub. @dseyler's PR is also failing on Mac with the same error.

@dcodoni
Copy link
Contributor

dcodoni commented Aug 26, 2025

@ktbolt I think the problem we are having with MacOS is related to the runner, when using the one built on 2025-07-11 it was working fine, the problem started to happen when we started to run on the one built in August:
Runner Image Provisioner
Hosted Compute Agent
Version: 20250818.377
Commit: 3c593e9f75fe0b87e893bca80d6e12ba089c61fc
Build Date: 2025-08-18T14:52:18Z

For this new built the CURRENT_OSX_VERSION is not automatically recognized and the problem is fixed if the following:
if(NOT DEFINED CURRENT_OSX_VERSION OR CURRENT_OSX_VERSION STREQUAL "")
execute_process(COMMAND sw_vers -productVersion
OUTPUT_VARIABLE CURRENT_OSX_VERSION
OUTPUT_STRIP_TRAILING_WHITESPACE)
endif()

Is added in SimVascularSystemSetup.cmake
I will open a new pull request with this little change.

@ktbolt
Copy link
Collaborator

ktbolt commented Aug 26, 2025

@dcodoni Very good detective work !

Please open an Issue for this describing how you found the problem CURRENT_OSX_VERSION is not automatically recognized and the solution, will definitely be useful to later developers.

@codecov
Copy link

codecov bot commented Aug 28, 2025

Codecov Report

❌ Patch coverage is 80.52493% with 371 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.90%. Comparing base (edee4b3) to head (76ea06a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Code/Source/solver/ris.cpp 42.32% 154 Missing ⚠️
Code/Source/solver/initialize.cpp 31.74% 43 Missing ⚠️
Code/Source/solver/uris.cpp 94.32% 36 Missing ⚠️
Code/Source/solver/Parameters.cpp 76.98% 29 Missing ⚠️
Code/Source/solver/all_fun.cpp 70.37% 24 Missing ⚠️
Code/Source/solver/output.cpp 46.66% 24 Missing ⚠️
Code/Source/solver/fluid.cpp 70.14% 20 Missing ⚠️
Code/Source/solver/distribute.cpp 94.17% 19 Missing ⚠️
Code/Source/solver/read_msh.cpp 95.23% 9 Missing ⚠️
Code/Source/solver/main.cpp 83.33% 6 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #421      +/-   ##
==========================================
+ Coverage   65.97%   66.90%   +0.92%     
==========================================
  Files         163      165       +2     
  Lines       31822    33711    +1889     
  Branches     5202     5672     +470     
==========================================
+ Hits        20996    22553    +1557     
- Misses      10688    11020     +332     
  Partials      138      138              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ktbolt ktbolt merged commit acc51f2 into SimVascular:main Aug 28, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge RIS valve implementation from Fortran to C++

4 participants