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][Core] Free degrees of freedom between stages #13099

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

rfaasse
Copy link
Contributor

@rfaasse rfaasse commented Feb 6, 2025

📝 Description
This PR aims to make the C++ process ApplyConstantScalarValueProcess more aligned to the python equivalents by releasing degrees of freedom after a stage. A unit test is added, as well as a documented integration test in the GeoMechanicsApplication to show the Dirichlet boundary condition is released in a second stage.

@rfaasse rfaasse requested a review from a team as a code owner February 6, 2025 14:49
@rfaasse rfaasse changed the title [GeoMechanicsApplication][Corefree degrees of freedom between stages [GeoMechanicsApplication][Core] Free degrees of freedom between stages Feb 6, 2025
@rfaasse rfaasse marked this pull request as draft February 6, 2025 14:49
@rfaasse rfaasse requested review from avdg81 and WPK4FEM February 6, 2025 14:53
@rfaasse rfaasse self-assigned this Feb 6, 2025
@rfaasse rfaasse requested review from markelov208 and removed request for a team February 6, 2025 14:57
markelov208
markelov208 previously approved these changes Feb 7, 2025
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.

Richard, thank you very much for the clear, documented and tested changes. As I told Copilot cannot improve your scripts. ;)

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

Richard, thank you for changes. The PR is good to be merged.

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.

Looks like a clear and well-tested fix to me. I have several minor suggestions, none of them being blocking. Thanks for the the work you've put into this.

@rfaasse rfaasse marked this pull request as ready for review February 11, 2025 12:40
@rfaasse
Copy link
Contributor Author

rfaasse commented Feb 11, 2025

@KratosMultiphysics/technical-committee This PR aims to fix the issue that was discussed earlier (for reference see the discussion in #12454). Feel free to only review the changes made in core (the changes in geo are reviewed within the team, but of course feedback is always welcome).

@rfaasse rfaasse requested a review from a team February 11, 2025 12:44
@rfaasse
Copy link
Contributor Author

rfaasse commented Feb 25, 2025

@KratosMultiphysics/technical-committee Would it be possible to have a look at this PR? We have discussed the problem before in #12454 (see comments) and it's a bug that impacts our geomechanics workflow.

PS. the geo/ prefix I only added to the branch name such that our own geo pipelines also get triggered, but the PR is really for core.

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.

[GeoMechanicsApplication] Degrees of freedom should be freed in between stages
3 participants