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

[GeoMechanicsApplication] Clean up and extract calculation of deformation gradients #12341

Merged
merged 13 commits into from
May 3, 2024

Conversation

avdg81
Copy link
Contributor

@avdg81 avdg81 commented May 3, 2024

📝 Description
Reduced the use of ElementVariables in the context of calculating the deformation gradients. This improves local reasoning. Furthermore, the code was simplified significantly, which eases understanding.

🆕 Changelog

  • Changed the signatures of members CalculateDeformationGradient:
    • They are no longer virtual, since no overrides were defined.
    • The return type has been changed to Matrix to return the result.
    • They no longer need a reference to data structure ElementVariables, since the result is now returned using the return type rather than a member of the output argument.
    • They have been made const, since no other members need to be modified.
  • Members CalculateDeformationGradient now only compute the deformation gradient itself, and no longer its determinant as well. To be on the safe side, the determinant is computed explicitly in a few places where it was not obvious whether the determinant is being used or not.
  • In several places, the use of ElementVariables had become redundant as a result of simplifying the code (i.e. after eliminating usages of members of ElementVariables). In most cases, we could use temporaries.
  • There is no need to explicitly check sizes before resizing a container: the resize operation of the container already takes care of that.
  • Removed two redundant calls to member CalculateKinematics (which only became apparent after carrying out the above changes).
  • Removed several comments that had no additional value.

avdg81 added 12 commits May 2, 2024 11:41
There were no `override`s provided for them.
Members `CalculateDeformationGradient` now return a `Matrix` object
rather than updating `ElementVariables::F`, which is passed as an output
argument to these functions.
- The members `CalculateDeformationGradient` have been made `const`.
- In member `CalculateAll`, compute the deformation gradients upfront.
If the determinant of the deformation gradient is needed (or when we are
not sure whether it is needed), compute it separately after computing
the deformation gradient.
Members `CalculateDeformationGradient` no longer use the
`ElementVariables` data structure, so it has been removed from their
interfaces.
Just `clear` the vector and `emplace_back` the elements that are to be
returned. Also removed a few comments that had no additional value.
Changes include:
- There was no need to use the `ElementVariables` data structure.
- Eliminated some redundant resizing.
- Removed a few comments that had no additional value.
When computing the Green-Lagrange strains, avoid storing temporary
results in data structure `ElementVariables`. Also removed some
unnecessary resize operations.
The `resize` operation itself checks whether the container already has
the desired size.
@avdg81 avdg81 added the GeoMechanics Issues related to the GeoMechanicsApplication label May 3, 2024
@avdg81 avdg81 requested review from rfaasse, WPK4FEM and markelov208 May 3, 2024 11:58
@avdg81 avdg81 self-assigned this May 3, 2024
For `UPwSmallStrainElement`, the deformation gradients were extracted
from the main loop in member `CalculateAll`. However, it did not replace
a call to `CalculateDeformationGradient` elsewhere. To actually do that,
member `CalculateStrain` also needs to be refactored. Since that is
beyond the scope of this PR, I have reverted the extraction.
Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

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

Looked together at this. To me it is consistent.

Comment on lines -144 to +145
rOutput.resize(mConstitutiveLawVector.size());

ElementVariables Variables;
this->InitializeElementVariables(Variables, rCurrentProcessInfo);

// Loop over integration points
rOutput.clear();
for (unsigned int GPoint = 0; GPoint < mConstitutiveLawVector.size(); ++GPoint) {
this->CalculateDeformationGradient(Variables, GPoint);
rOutput[GPoint] = Variables.detF;
rOutput.emplace_back(MathUtils<>::Det(this->CalculateDeformationGradient(GPoint)));
Copy link
Contributor

Choose a reason for hiding this comment

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

With the removal of InitializeELementVariables this functionality may have changed from an initial configuration F_0 to a current cofiguration F.

This is a wrong interpretation. It would lead to F = I

Together with Anne we looked through.

Copy link
Contributor

@markelov208 markelov208 left a comment

Choose a reason for hiding this comment

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

Anne, a very good work. A lot of redundant lines are removed. I have made comments mainly for my knowledge.

Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

This looks like a nice and clear clean-up. No further comments from my side! I didn't go through all the functions to see whether any state of ElementVariables could lead to diffs, but since you and @WPK4FEM already looked at this, that shouldn't block this PR.

Copy link
Contributor

@markelov208 markelov208 left a comment

Choose a reason for hiding this comment

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

Many thanks for the clarifications

@avdg81 avdg81 merged commit ac6f95d into master May 3, 2024
11 checks passed
@avdg81 avdg81 deleted the geo/extract-deformation-gradients branch May 3, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GeoMechanics Issues related to the GeoMechanicsApplication
Projects
None yet
4 participants