Skip to content

Conversation

@JRao-rgb
Copy link

@JRao-rgb JRao-rgb commented Nov 16, 2023

Current situation

#110
I found out the linear material model is broken when I was testing the backwards compatibility of the leaky vessels code with previous sv input files. I initially thought it was my code that was broken, but the code just didn't handle linear materials properly.

Release Notes

  • Changed the following functions in cvOneDMaterialLinear.cxx: GetArea(), GetPressure(), GetDpDS(). Not sure how the previous maintainer arrived at these forms, but they differ a lot from the implementation in cvOneDMaterialOlufsen.cxx, which doesn't make much sense (since the two models are only different in the EHR computation).
  • Changed the function GetN() in cvOneDMaterialLinear.h from "return 0.0;" to "return N;".
  • Added relevant test cases so that the linear material can be tested when future versions of the code are changed.

Documentation

  • Just a bug fix.

Testing

Please ensure that the PR meets the testing requirements set by GitHub Actions.

  • See above for testing results. The linear material should undergo tests now.

Code of Conduct & Contributing Guidelines

@JRao-rgb
Copy link
Author

@mrp089 should be good to merge! Please take a look when you get time.

@mrp089 mrp089 self-requested a review November 17, 2023 02:32
Copy link
Member

@mrp089 mrp089 left a comment

Choose a reason for hiding this comment

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

Thank you so much for doing this, @JRao-rgb! Please see my comments and ask me if you have any questions. Congratulations on your first pull request!

Copy link
Member

Choose a reason for hiding this comment

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

Check out this file from master branch because there are only formatting changes in here

Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to do one of these tests with " $Eh/r \ll\infty$ " for Olufsen and Linear material? Then, we should see that they deform similarly. Currently, So_/S would always be close to one, right?


void cvOneDMaterialLinear::SetEHR(double ehr_val, double pref_val){
ehr = ehr_val;
ehr = 4.0/3.0*ehr_val; // JR 15/11/23: multiplied EHR by the correct factor (since downstream analysis using EHR
Copy link
Member

Choose a reason for hiding this comment

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

No need to add date and initials (even though people did it before), that's now all stored in git. You can check it out by doing a git blame on a file.

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.

2 participants