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

Ustruct and volumetric material tests #229

Conversation

aabrown100-git
Copy link
Collaborator

@aabrown100-git aabrown100-git commented Jul 28, 2024

Current situation

Material unit tests do not test ustruct material model implementations, nor volumetric penalty model implementations. #150

Release Notes

  • Add/update comments in material testing code and in material model implementation code
  • Add instructions for running material tests
  • Add tests for ustruct material model implementations
  • Add tests for volumetric penalty model implementations (for both struct and ustruct)
  • Fix bug in ustruct HO model where PK2 stress was not being updated

Testing

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

This code itself is testing.

Code of Conduct & Contributing Guidelines

Copy link

codecov bot commented Jul 28, 2024

Codecov Report

Attention: Patch coverage is 95.05208% with 19 lines in your changes missing coverage. Please review.

Project coverage is 65.72%. Comparing base (f424b7c) to head (69eed1b).

Files Patch % Lines
tests/unitTests/test.h 78.16% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
+ Coverage   64.96%   65.72%   +0.76%     
==========================================
  Files         116      116              
  Lines       29153    28876     -277     
==========================================
+ Hits        18938    18980      +42     
+ Misses      10215     9896     -319     

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

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.

@aabrown100-git, I'm very happy you tackled testing the solid material models!

As someone who has messed up a test-PR before, I have one piece of wisdom: Do your tests fail if you change parts of the code that the test should be sensitive to ("test the test")? You can verify this locally.

Also, thank you for adding comments to your (and existing code)! You could slightly change their format and they would show up in Doxygen. There's a quick guide here (it's mostly converting to /// or @brief). There's also a plugin for VS Code that makes it very easy (probably for other editors as well).

Code/Source/svFSI/mat_models.cpp Show resolved Hide resolved
tests/unitTests/test.h Outdated Show resolved Hide resolved
tests/unitTests/test.h Outdated Show resolved Hide resolved
tests/unitTests/test.h Show resolved Hide resolved
@MatteoSalvador
Copy link
Collaborator

Thanks @mrp089 for your comments!
@aabrown100-git, once all the points raised by @mrp089 are addressed I will merge this PR!

@aabrown100-git
Copy link
Collaborator Author

@mrp089 Thanks for the comments!

  • I was able to make all the tests fail, so I think the tests are tested :)
  • I just modified many of the comments to cooperate with Doxygen.
  • I responded to your other comments individually.

@MatteoSalvador MatteoSalvador merged commit c487190 into SimVascular:main Aug 2, 2024
5 checks passed
@aabrown100-git aabrown100-git deleted the ustruct_and_volumetric_material_tests branch August 2, 2024 19:21
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.

3 participants