-
Notifications
You must be signed in to change notification settings - Fork 27
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 first unit test #150
Comments
@mrp089 As I recall I said let's not do this because it is a lot of work and the code was not implemented with unit testing in mind. There are lots of other more important things that need to be done. If @yuecheng-yu has some time then he can work on #30 or update our material models with those of svFSI. |
Reopening this after talking with @ktbolt, so we have a place to discuss and exchange ideas. My PhD lab is using GoogleTest (or gtest) in their (closed-source) multi-physics FEM code. Here is their 30 min video guide how to include GoogleTest (slides). This was part of a Summer of Testing series. Here is a quick start on including it using CMake. A challenge in the current |
@yuecheng-yu has prototyped a first unit test using GoogleTest and calling |
@mrp089 I think I find the "secrete" location of Google Test. Finally, I don't choose to separate the unit test's CMakeLists, because it never works at any directories (as we discussed today). Instead, the main program's CMakeLists (.../svFSI/CMakeLists.txt) extends to build both Google Test and Unit Test. During compilation, after the test cases (hello_test.cpp, test.cpp) built, Google Test will "pre-run" all of them from the directory: |
I tried to replace |
Google Mock might be a way to get around reading an input file to create a ( |
@mrp089 The idea is to make the unit tests as simple and independent of other objects as possible, don't want to need to modify a bunch of unit tests when a class changes. And you don't want to read in files. Creating mock objects is a way to implement this, most unit test frameworks support creating mock objects. |
I've started expanding the unit testing for material models a bit. You can find the latest commits here: https://github.com/aabrown100-git/svFSIplus/tree/material_unit_tests I wrote tests to check the consistency of the Mooney-Rivlin 2nd PK stress TEST 1: see
|
I think that's an excellent idea, @aabrown100-git! I'd be happy to help make this a pull request. I have two ideas:
Let me know what you think! |
@aabrown100-git A good start. Some comments:
|
Thanks @mrp089 and @ktbolt. I've made a few changes based on your suggestions. Currently, the code is a bit disorganized, but I think it will become cleaner after we make some decisions. @mrp089 Regarding your two ideas
|
I've reorganized the material model tests and added a variety of tests for the Holzapfel-Ogden material model. On my computer, these tests pass, but they sometimes fail due to randomness in generating test deformation gradient tensors. Following @mrp089 suggestion, I will think about how to test the rate of convergence of the finite difference approximations instead. I implemented these tests primarily to verify the Holzapfel-Ogden material model implementation for a cardiac mechanics benchmark problem I want to solve with svFSIplus. I think the tests are currently sufficient for that. @ktbolt @mrp089 I think merging it with svFSIplus would be useful, but we may also want to wait until @lpapamanolis finishes the material model restructuring. If we want to merge it soon, please look over the code when you get a chance and let me know what can be improved. |
@aabrown100-git You should be using the C++ random library and not If the test are useful then I say go ahead and merge unless this breaks something. |
@ktbolt Thanks for the advice Dave, I just modified the code to switch to the C++ random library. I'll submit a merge and we can continue the conversation there! |
I've added some more tests (testing ustruct material model implementations and testing volumetric penalty model implementations for both struct and ustruct). I'll submit a new merge request. |
I'm expanding the tests to test the order of convergence of the finite difference estimates for S and CC. You can find the latest here. One issue I'm dealing with is that as delta decreases, the error decreases as expected, but eventually it bottoms out around 1e-6~1e-7 (due to round-off error?). I am computing the order of convergence by finding the slope of log(error) vs. log(delta), so if this bottomed-out error is included the order of convergence is computed incorrectly. Is there some way to ignore this bottomed-out error when calculating order of convergence? Also, any ideas why the error bottoms out so high (in double precision shouldn't it bottom out around 1e-16?). |
@aabrown100-git It's not clear to me what the motivation is for adding more unit tests. With 50 open svFSIplus Issues I have to think that there are more important things to work on than adding another unit test that will in future need to be refactored. But if you think this is important then create a new Issue for it so it can be more clearly tracked. |
@ktbolt I'm interested in improving the unit tests to have confidence in the material model implementations, especially because I and others want to run some cardiac mechanics benchmark cases in svFSIplus. I'll create a new issue for this. |
I think perhaps we can close this issue? |
@aabrown100-git Gaining confidence in the material models is definitely a good thing, have at it! |
Problem
When implementing new features, having unit tests would be very useful. This is especially true for solid material models since they have a clearly defined interface:
It's also straight-forward to come up with analytical solutions for materials to test against.
Solution
We will add a single unit test, starting with the Neo-Hookean material under different loading conditions. Once we have a framework integrated into GitHub actions, we can expand it to other currently implemented hyperelastic materials (since we need to cover them anyway). Finally, we can use that framework to add new material models (#2, #33, #49).
Additional context
@yuecheng-yu will add a
.cpp
and acmake
file. @mrp089 will integrate the executable into GitHub actions. @MatteoSalvador, input welcome!Code of Conduct
The text was updated successfully, but these errors were encountered: