Skip to content

[GeoMechanicsApplication] Removing commented code and changing typedef -> using#12369

Merged
rfaasse merged 2 commits intomasterfrom
geo/remove-commented-code-custom_utilities
May 13, 2024
Merged

[GeoMechanicsApplication] Removing commented code and changing typedef -> using#12369
rfaasse merged 2 commits intomasterfrom
geo/remove-commented-code-custom_utilities

Conversation

@rfaasse
Copy link
Contributor

@rfaasse rfaasse commented May 10, 2024

During the formatting of the custom elements, we found quite some commented code which we would like to remove. As a quick win, I also changed typedefs to usings (and removed some unused ones) in the files I touched anyway.

@rfaasse rfaasse requested review from avdg81 and markelov208 May 10, 2024 14:58
@rfaasse rfaasse self-assigned this May 10, 2024
}

if (p < 0)
return false; // in this case the square roots below will be negative. This substitutes with better efficiency lines 107-110
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one I changed since due to the clang-formatting the lines shifted

*
*/

class GeoStaticCondensationUtility
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems this was copied from structural mechanics, we might just be able to re-use that one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please :-)

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

Hi Richard, thank you for the quick improvements. Just one comment on Portuguese. Is it a correct translation? Kind regards,

@@ -712,8 +667,8 @@ class GeoMechanicsMathUtilities
static inline void TensorToMatrix(Fourth_Order_Tensor& Tensor, Matrix& Matrix)
{
// Simetrias seguras
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall it be 'Safe symmetries'? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to translate the meaning, good point!

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

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

This is a small, but clear PR that reduces the amount of clutter and dirt. Thank you for taking this up.

using VectorType = Vector;
using IndexType = unsigned int;
using SizeType = unsigned int;
using Fourth_Order_Tensor = DenseVector<DenseVector<Matrix>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps for consistency, we should rename this alias to FourthOrderTensorType?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would propose to remove all unused functions from this file after you have merged this PR.

*
*/

class GeoStaticCondensationUtility
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, please :-)

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

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

I feel this one is good to go.

@rfaasse rfaasse merged commit 0435380 into master May 13, 2024
@rfaasse rfaasse deleted the geo/remove-commented-code-custom_utilities branch May 13, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants