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.

Sign up for GitHub

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

Jump to bottom

Add processing the Holzapfel material model. #131

Closed
wants to merge 149 commits into from
Closed

Add processing the Holzapfel material model. #131

wants to merge 149 commits into from

Conversation

ktbolt
Copy link
Collaborator

@ktbolt ktbolt commented Nov 2, 2023

This is for #103.

Note that the current material models in svFSIplus are different from svFSI.

ktbolt and others added 30 commits December 15, 2022 11:04
mrp089 and others added 9 commits September 21, 2023 10:59
@mrp089
Fix deployment if statement (#104)
ffe6335
@mrp089
fix links in readme (#108)
1b6e75d
@ktbolt @MatteoSalvador
Add printing valid material model types in the error message. (#106)
f7f99dc 
Co-authored-by: Matteo Salvador <[email protected]>
@elenasmartinez
Fluid and ustruct test cases (#115)
c14486e 
Co-authored-by: Elena Martinez <[email protected]>
@MatteoSalvador
ustruct tests (P1P1-VMS block compression, tensile adventitia HGO, LV…
6922927 
… guccione active) (#113)
@wgyang @mrp089
Add a CMM test case (#117)
9723a97 
* Add a CMM test case. Results are consistent with svFSI output. Prestress examples will be added.

* Use a linear solver setting without Trilinos for the cmm inflation test case.

* Remove .inp files in the cmm test case. Reduce the number of time steps for the inflation procedure.

* Add prestress examples for the cmm test case.

---------

Co-authored-by: Martin R. Pfaller <[email protected]>
@MatteoSalvador
Copyright workflow and licensing (#122)
eec4bd7
@MatteoSalvador
Bueno-Orovio parameters (#130)
d18a1fe 
* Expose tau_fi and tau_si from BO in the XML interface

* Tests
@ktbolt
Add processing the Holzapfel material model.
d0fe09e
@ktbolt ktbolt requested a review from kharold23 November 2, 2023 23:01
@ktbolt
Copy link
Collaborator Author

ktbolt commented Nov 3, 2023

@kharold23 Merging still blocked, try approving again please.

kharold23
kharold23 approved these changes Nov 3, 2023
View reviewed changes
Copy link
Contributor

@kharold23 kharold23 left a comment

Choose a reason for hiding this comment

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

Changes look good!

@kharold23
Copy link
Contributor

Hmm... I've tried approving it 4 times. Not sure why it still says 1 review required...

@kharold23
Copy link
Contributor

Do I have write access? It says "At least 1 approving review is required by reviewers with write access", so maybe that's why mine aren't working?

@ktbolt
Copy link
Collaborator Author

ktbolt commented Nov 3, 2023

@kharold23 Ah yes, you need to be an. owner. I will have @MatteoSalvador review it.

MatteoSalvador
MatteoSalvador approved these changes Nov 3, 2023
View reviewed changes
Copy link
Collaborator

@MatteoSalvador MatteoSalvador left a comment

Choose a reason for hiding this comment

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

Looks good to me @ktbolt! However, before merging the PR, I would kindly ask @aabrown100-git to add a test for the Holzapfel model from one the benchmark, if possible.

@ktbolt
Copy link
Collaborator Author

ktbolt commented Nov 3, 2023

Yes, please add a test case.

@aabrown100-git
Copy link
Collaborator

What type of test would be useful? I can create a test to compare the svFSIplus Holzapfel model to the old svFSI Holzapfel model (from around Nov. 2021), but this has bugs that have since been fixed.

Here is a test case to compare the HO-ma model currently in svFSI to the eventual implementation in svFSIplus:
HO_passive_inflation.zip

Let me know what is most useful, and I can modify it accordingly!

@MatteoSalvador
Copy link
Collaborator

Thanks @aabrown100-git! I think that the HO-ma_passive_inflation test you sent is a good choice. We will change the reference solution in the near future when all the bug fixes will be incorporated in svFSIplus as well

@mrp089 mrp089 mentioned this pull request Nov 30, 2023
Closed
1 task
@mrp089 mrp089 closed this Nov 30, 2023
@mrp089
Copy link
Member

mrp089 commented Nov 30, 2023

Apparently, this got closed automatically by my force push and can't be reopened.

@aabrown100-git, can you please do the following (some of which you might have done already):

  • Create a branch in your fork to track these changes
  • Apply the migration outlined here
  • Make your additional edits
  • Open a new pull request

I appreciate your help, and I'm sorry for the extra trouble!

@aabrown100-git aabrown100-git mentioned this pull request Dec 1, 2023
Closed
1 task
@aabrown100-git aabrown100-git mentioned this pull request Dec 13, 2023
Merged
1 task
MatteoSalvador pushed a commit that referenced this pull request Dec 13, 2023
@aabrown100-git
Holzapfel material model and test (#164)
90c27b8 
* Add processing the Holzapfel material model.

Copy commits to process Holzapfel material model from Dave, originally in #131

* Adding LV_Holzapfel_passive test case

* Adding .bin and .stl to .gitattributes for LFS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MatteoSalvador MatteoSalvador MatteoSalvador approved these changes

@kharold23 kharold23 kharold23 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ktbolt @kharold23 @aabrown100-git @MatteoSalvador @mrp089 @fgerosa @zinanhu0810 @wgyang @elenasmartinez