Closed bodhinandach closed 3 years ago
Merging #698 into develop will increase coverage by
0.07%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## develop #698 +/- ##
===========================================
+ Coverage 96.69% 96.77% +0.07%
===========================================
Files 130 130
Lines 25823 25834 +11
===========================================
+ Hits 24969 24999 +30
+ Misses 854 835 -19
Impacted Files | Coverage Δ | |
---|---|---|
include/particles/particle.tcc | 95.21% <100.00%> (+3.27%) |
:arrow_up: |
tests/particle_serialize_deserialize_test.cc | 100.00% <100.00%> (ø) |
|
include/particles/particle_base.h | 100.00% <0.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update e98e589...f39f42d. Read the comment docs.
Thanks @bodhinandach Great catch!
@kks32 I spent the whole weekend to debug this :sob: but I feel good after fixing it :smile: Thanks for the quick review @kks32
Describe the PR I found two major bugs in the new particle deserialization function. The first one is that
mass_density_
is not computed from mass and volume as in hdf5 initialization, and therefore equals0
. The navier stokes and two phase solvers will have an error, while since it is not used in the explicit solver, we didn't realize this bug. The second one is, if material with state variables is used, the current version of deserialization cannot initialize thesvars
properly in the new particle. We need to add the [0] while unpacking the svars vector. The current test and benchmarks are using linear elastic, and therefore, this error was not detected, however, mohr-coulomb problems show a segmentation fault as attached below.Related Issues/PRs PR #689
Additional context See the screenshot before and after the modification of the
After:
![image](https://user-images.githubusercontent.com/37140224/94428895-b14c1e80-01bb-11eb-8431-8275e60a967a.png)
svars
below (at step 8975). This is a Mohr-Coulomb dam break problem. Before:I added tests for both the mass density and the state variables, i.e. changing from LinearElastic3D to Newtonian3D that has pressure as the state variables.