ikarus-project / ikarus

Ikarus is a C++-based library that uses DUNE modules to solve partial differential equations with the finite element method and more.
https://ikarus-project.github.io/
Other
6 stars 3 forks source link

Use AutoDiffScalar Concept and fix checkFESByAutoDiff test #329

Closed henrij22 closed 1 month ago

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.86%. Comparing base (8e2263e) to head (492863b). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #329 +/- ## ========================================== - Coverage 86.66% 80.86% -5.80% ========================================== Files 69 68 -1 Lines 2234 2299 +65 ========================================== - Hits 1936 1859 -77 - Misses 298 440 +142 ``` | [Flag](https://app.codecov.io/gh/ikarus-project/ikarus/pull/329/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ikarus-project) | Coverage Δ | | |---|---|---| | [tests](https://app.codecov.io/gh/ikarus-project/ikarus/pull/329/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ikarus-project) | `80.86% <100.00%> (-5.80%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ikarus-project#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tarun-mitruka commented 2 months ago

This action still failed irrespective of the fact that no floating-point exceptions were raised in this commit(note that this failure is not attributed from this PR). The tests are hence not yet stable and I would recommend to wait before merging this branch and finding a fix before, even though it looks like the CI works at the moment

rath3t commented 1 month ago

This action still failed irrespective of the fact that no floating-point exceptions were raised in this commit(note that this failure is not attributed from this PR). The tests are hence not yet stable and I would recommend to wait before merging this branch and finding a fix before, even though it looks like the CI works at the moment

The 3d test does not touch the vanishing stress code or did you find out something different?

The test now runs locally without any exceptions from the floating point?

tarun-mitruka commented 1 month ago

This action still failed irrespective of the fact that no floating-point exceptions were raised in this commit(note that this failure is not attributed from this PR). The tests are hence not yet stable and I would recommend to wait before merging this branch and finding a fix before, even though it looks like the CI works at the moment

The 3d test does not touch the vanishing stress code or did you find out something different?

No, it doesn't. At least for this commit, all the actions were successful.

tarun-mitruka commented 1 month ago

OBSERVATIONS: Here, I am documenting the current observations:

  1. autoDiffTest<3>(/* ... */) fails ONLY for clang compiler when using CornerDistortionFlag::randomlyDistorted with nu !=0 and d.setRandom(nDOF)
  2. autoDiffTest<3>(/* ... */) fails for BOTH clang and gcc compilers when using CornerDistortionFlag::unDistorted with nu !=0 and d.setRandom(nDOF) (see here)

The test fails irrespective of the number of minimum iterations (see here) allowed and presence of perturbation the stresses (see here) in vanishingstress.hh

Further debugging is necessary!!

!!! UPDATE The autodiff<gridDim>(/* ... */) test executed immediately after autoDiffTest<2>(t, planeStressMat2, " nu = 0"); fails, irrespective of gridDim and nu, but only for non-zero displacements. with CornerDistortionFlag::unDistorted.

rath3t commented 1 month ago

Since it only fails if the tests are executed simultaneously, it sound still like an uninitialized memory problem or similar. Therefore, I would suggest executing the test with valgrind to check for issues.

I would suggest, the following procedure, if the 3D-test would fail independently

  1. Extract some configuration (node positions+ displacements) for which the tests fail
  2. Modify the test to explicitly check for the same configuration
  3. Check if NeoHooke and StVenantKirchhoff, become the same for displacements going to zero and Poisson's ratio going to zero
  4. Reduce the formulation to a single integration point
  5. Extract the corresponding strain for which the test fails
  6. Create a new test without any element but only with the material and check if energy,stresses and tangent moduli are correct/the same for autodiff and implementation (This test should already exists somewhere)
tarun-mitruka commented 1 month ago

I think I found the source of the error. I ran the tests 7 times and it always works now (see this action run).

CornerDistortionFlag::randomlyDistorted adds distortions in the range [-0.2, 0.2] to the nodal positions of the reference cube element. (see here). d.setRandom(basis.flat().dimension()) creates a vector with entities in the range [-1, 1] (from Eigen). This can sometimes leads to a heavily distorted element with a negative deformation gradient. This breaks the sqrt function here in NeoHooke as it is not defined for negative values. I think depending on the randomness, the observations I mentioned here was a special case of the above mentioned issue (undefined behavior). I now scaled the displacements in checkFESByAutoDiff such that the random values in the displacement vectors are in the range [-0.2, 0.2] and also added checks in NeoHooke such that it throws a proper error message when detC < 0. May be this can be scaled down further for extreme case scenarios?

rath3t commented 1 month ago

that makes a lot of sense. I will look into it tomorrow.

tarun-mitruka commented 1 month ago

The code coverage is also behaving randomly, I don't know why. Before I committed the change from MathError to InvalidStateException, codecov/project increased by +2.01% and after the commit it decreased with -6.36%. How can this happen?

rath3t commented 1 month ago

The code coverage is also behaving randomly, I don't know why. Before I committed the change from MathError to InvalidStateException, codecov/project increased by +2.01% and after the commit it decreased with -6.36%. How can this happen?

This is also a mystery for me, therefore I take these changes always with a grain of salt. We might remove that even or change to another provider

rath3t commented 1 month ago

In total for me this looks quite fine and the comments might also happen in another PR except for the code duplication stuff. Still you can quickly look into if you have an idea that can be done fast. Thanks Tarun for investigation!