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] Add a utility function for calculating the internal force vector #12669

Merged
merged 13 commits into from
Sep 11, 2024

Conversation

avdg81
Copy link
Contributor

@avdg81 avdg81 commented Sep 10, 2024

📝 Description

Added a utility function for calculating the internal force vector. As input, it requires vectors of B-matrices, stress vectors, and integration coefficients (one for each integration point). When the input vectors have different sizes or when they are empty, an error is raised.

The internal force vector is computed as follows:

$$\int_{\Omega_e} \mathrm{B}^T \sigma \mathrm{d} \Omega$$

@avdg81 avdg81 added the GeoMechanics Issues related to the GeoMechanicsApplication label Sep 10, 2024
@avdg81 avdg81 requested review from rfaasse and WPK4FEM September 10, 2024 12:12
@avdg81 avdg81 self-assigned this Sep 10, 2024
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.

Dear Anne,
Nice and short functionality. We could also test further for size consistency within the vectors of b matrices and stress vectors.
Thank you,
WIjtze Pieter

GeoEquationOfMotionUtilities::CalculateInternalForceVector(b_matrices, stress_vectors, integration_coefficients),
"Cannot calculate the internal force vector: input vectors have different sizes")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

All Matrices in the b_matrices should have the same sizes. This also hold for the vectors in stress vectors.
Further the stress_vector size should be equal to the first size of a b matrix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I have added checks for these preconditions that are only carried out by debug builds.

rfaasse
rfaasse previously approved these changes Sep 10, 2024
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.

Could not find anything to comment on (although it would've been nice if a transform with 3 ranges would exist.

…DEBUG_ERROR_IF`

The tests that cover these checks are now conditionally built and run, too.
rfaasse
rfaasse previously approved these changes Sep 11, 2024
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.

Looks good to me! I have 2 minor points, but feel free to merge without addressing them

}

// The following tests only raise errors when using debug builds
#ifdef KRATOS_DEBUG
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we think these checks would ever be helpful for cases which are run in release? I like that this way it doesn't impact release performance, but I'm just putting the question here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the input for GeoEquationOfMotionUtilities::CalculateInternalForceVector is prepared by us (the programmers) and any inconsistencies in the input data are expected to affect previous operations, @WPK4FEM and myself decided that a debug check is adequate here. It's more like an assertion: this is never supposed to happen, but we do want to catch it at debug-time if it ever happens. As you wrote yourself, the end-user (who uses a release build) doesn't pay the price of the additional checks, which benefits general performance.

const auto integration_coefficients = std::vector<double>{0.25, 0.4};

KRATOS_EXPECT_EXCEPTION_IS_THROWN(
GeoEquationOfMotionUtilities::CalculateInternalForceVector(b_matrices, stress_vectors, integration_coefficients), "Cannot calculate the internal force vector: matrix-vector product cannot be calculated due to size mismatch")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a very long line, I expect it needs to still be formatted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... no. For some unclear reason, clang-format doesn't break this long line. When I manually insert a line break between the function call and the expected error text, clang-format will revert it to the original long line...

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.

Thank you for the revisions. Good that regular users are not confronted with the checking as it is our responsibility to ensure passing of correct vectors and matrices.
Good to go.

@avdg81 avdg81 enabled auto-merge (squash) September 11, 2024 08:45
@avdg81 avdg81 merged commit 01c86af into master Sep 11, 2024
9 of 11 checks passed
@avdg81 avdg81 deleted the geo/extract-calculate-internal-force-vector branch September 11, 2024 09:10

auto result = Vector{ZeroVector{rBs.front().size2()}};
for (auto i = std::size_t{0}; i < rBs.size(); ++i) {
result += prod(trans(rBs[i]), rStressVectors[i]) * rIntegrationCoefficients[i];
Copy link
Member

Choose a reason for hiding this comment

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

i think that with noalias(result) += should be faster

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
Development

Successfully merging this pull request may close these issues.

4 participants