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.

Sign up for GitHub

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

Jump to bottom

Geo/11721 fix settlement output #11728

Merged
merged 102 commits into from
Oct 28, 2023
Merged

Geo/11721 fix settlement output #11728

merged 102 commits into from
Oct 28, 2023

Conversation

rfaasse
Copy link
Contributor

@rfaasse rfaasse commented Oct 26, 2023 •
edited
Loading

📝 Description
This PR aims to:

  1. Fix that the results after applying a normal load were different from the results when running the settlement through the python interface
  2. The results after applying excavation now write 0s for displacements on excavated nodes (like in the python workflow)
  3. The settlement outputs for all time steps are now written to file, instead of only the last one.

WPK4FEM and others added 30 commits October 9, 2023 13:19
@WPK4FEM
WPK first attempt time loop executor
8c1fa6a
@WPK4FEM
also complementary to WPK first attempt time loop executor
294c2ff
@WPK4FEM
Removed yet unused things. Added first unit test: TimeLoopReturnsPerf…
bdcadbc 
…ormedStatesAfterRunning. Removed template argument from TimeLoopExecutor class.
@WPK4FEM
Added a StrategyWrapper with an always converging solver strategy for…
9f47ad5 
… testing
@WPK4FEM
Added constructor with convergence argument to DummySolverStrategy. A…
0bfebf1 
…dded test for running and terminating a loop with a non-converging solver strategy.
@WPK4FEM
Give time loop executor a TimeStepEndState to start from. Keep track …
afb17c5 
…of the time passed. Added a test on end time after N steps.
@WPK4FEM
Use the time step executor. ( only one available so default construct…
81000b3 
…ed ). Passed solver strategy to time step executor.
@WPK4FEM
Keep track of used number of cycles. Added tests using a test time in…
767fb7c 
…crementor with a fixed number of cycles per step. Added RunCycle and RunCycleLoop to the time loop executor.
@avdg81
Removed a few unused dummy classes for testing
5476cbd 
Also added the `explicit` keyword to a single-argument constructor.
@avdg81
Test the time loop executor using a prescribed time incrementor
e74ad60 
Given a prescribed time incrementor that keeps four time increments and
a solver strategy that always converges, we expect to get four converged
time step end states, where each one just required a single cycle. Also
the time values are validated for each time step end state.

Furthermore added a helper function to build a converged end state.
@avdg81
Added the strategy wrapper as a member of the time loop executor
3f491e8 
Now that the strategy wrapper is a member of the time loop executor, we
can set the end time and the time increment in their "global" variables.
Internally, this is done via the model part. The reason we set these
variables explicitly is that processes may rely on having this
information available.
@avdg81
When running the time loop, increment the step number
712c6c0 
Extended the API of class `StrategyWrapper` such that we can increment
the step number and get the step number upon request.
@WPK4FEM
Added test for and implementation of CloneTimeStep, in the strategy w…
79147cd 
…rapper needed for the time loop executor.
@rfaasse
Merge branch 'master' into settlement_test
ba64d48
@WPK4FEM
Added method and tests for RestorePositionsAndDOFVectorToStartOfStep …
916242f 
…for restoring to the initial situation of a step to enable cycling of the step.
@rfaasse
First implementation of the strategy wrapper implementation
6bc0546
@rfaasse
Added unit tests for all non-trivial functionality of the wrapper
10cab94
@rfaasse
Fix pipeline build
4b42c62
@WPK4FEM
SaveTotalDisplacementFieldAtStartOfStage and AccumulateTotalDisplacem…
9753302 
…entField implemented and unit tests provided.
@WPK4FEM
OutputProcess implementation and unit test added to time_loop_executor.
9ba7a26
@WPK4FEM
FinalizeSolutionStep should be called outside the cycle loop. This ch…
f98bb53 
…anges a test for the TimeStepExecutor and there is a new one for the TimeLoopExecutor.
@WPK4FEM
Small things from review by Richard adapted.
19d600f
@WPK4FEM
Pass on process observables from TimeLoopExecutor to TimeStepExecutor.
90a341d
@WPK4FEM
Less code repetition in test_time_loop_executor
46b2597
@WPK4FEM
interface class for TimeLoopExecutor as requested in review by Richard.
c071354
@WPK4FEM
merge from master
16eb498
@rfaasse
Added all the new strategy wrapper functions
a0feda0
@WPK4FEM
forgotten interface.h
b832147
@WPK4FEM
Code smell remarks from SonarCloud almost all resolved
588b0ed
@rfaasse
Added UTs for Getters, setters and Save and Accumulate
5795628
avdg81 and others added 19 commits October 23, 2023 11:15
@avdg81
Reordered a few lines to make the flow more logical
fd90b0e
@rfaasse
Removing SQ issues
b57a5b6
@rfaasse
Merge remote-tracking branch 'origin/geo/strategy_wrapper_implementat…
9c5b4c1 
…ion' into geo/strategy_wrapper_implementation
@avdg81
Two minor enhancements
9b119b2 
1. Prefer to use the pre-increment operator rather than the
   post-increment operator.
2. Made a local variable `const`.
@avdg81
Removed several log statements
1cb6b02
@avdg81
Removed a redundant statement
5084658 
A node that has been created by a model part does not need to be added
explicitly to that same model part later on.
@avdg81
Removed a redundant destructor
362af55
@rfaasse
Added test for saving total replacements twice and other minor fixes
cd1e427
@rfaasse
Fix Codacy complaint
cc0cc0a
@rfaasse
Fix UT
708c35c
@rfaasse
Only run stage 1-3 in the workflow test, since stage 4 has known issues
3522d0f
@rfaasse
Merge branch 'master' into geo/strategy_wrapper_implementation
2c3373a
@rfaasse
Possible fix for pipeline test failure
60492ee
@rfaasse
Another attempt at fixing the pipeline test
c33b4de
@rfaasse
Skipping workflow test temporarily due to discussed pipeline issues
f6d565b
@rfaasse
Fixing normal load
7018f20
@rfaasse
Merge branch 'master' into geo/11721-fix-settlement-output
8a86b4e
@rfaasse
Minor change to test and fixed the displacement for excavated part of…
0ddff31 
… model
@rfaasse
Fixed writing multiple time steps in one file
6a73f78
@rfaasse rfaasse linked an issue Oct 26, 2023 that may be closed by this pull request
[GeoMechanicsApplication] Fix settlement output for normal load and inactive nodes/excavation #11721
Closed
rfaasse
rfaasse commented Oct 26, 2023
View reviewed changes
applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.cpp Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/solving_strategy_wrapper.hpp Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/solving_strategy_wrapper.hpp Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/write_output.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_settlement_workflow.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/test_settlement_workflow/MaterialParameters.json Show resolved Hide resolved
...cations/GeoMechanicsApplication/tests/test_settlement_workflow/ProjectParameters_stage4.json Show resolved Hide resolved
...tions/GeoMechanicsApplication/tests/test_settlement_workflow/test_model_stage4.post.orig.res Outdated Show resolved Hide resolved
@avdg81
Code cleanup and minor corrections
cd2ad1b 
- Prefer to use curly braces when constructing an object.
- Removed a comment that was no longer correct due to changes.
- Moved the definition of `GeoOutputWriter`'s constructor to the top of
  the file.
- Only pass the GiD output settings to `GeoOutputWriter`'s constructor
  and its member function `WriteGiDOutput`, since we only need settings
  from that section of the project parameters. This simplifies the
  implementation of both member functions.
- Removed an unused local variable.
- Removed a redundant local variable.
- Removed an unused function parameter.
- Removed several redundant function parameters.
- Use more compact notation to get an array item.
- Changed some private member functions to local helpers (to clean up
  `GeoOutputWriter`'s interface).
- Pass a function by reference to const rather than by value.
- Extracted a helper function to create a `GidIO<>` object.
- Extracted member functions that return the main model part and the
  computational model part.
- Extracted a member function that restores the values of the given
  nodal variable.
- The `ROTATION` variable is now also restored when it is present.
- Added an apparently missing update to the initial configuration.
- Simplified some pieces of code.
- A few minor style changes.
@avdg81
Updated the settlement workflow "unit test"
8d08a1b 
- Reverted the changes that actually run the test and where we log to a
  local file. If we don't do that, the build checks on GitHub fail for
  unknown reasons. When we run the test locally or on our own build
  server all is fine.
- Increased the maximum stage number in the test from 3 to 4.

Furthermore, a minor style change has been made, and a path has been
reverted in a Python script.
avdg81
avdg81 approved these changes Oct 27, 2023
View reviewed changes
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.

Really nice and clean changes. Thanks Richard!

applications/GeoMechanicsApplication/custom_workflows/write_output.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.cpp Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/solving_strategy_wrapper.hpp Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/solving_strategy_wrapper.hpp Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.hpp Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/test_settlement_workflow/MaterialParameters.json Show resolved Hide resolved
...cations/GeoMechanicsApplication/tests/test_settlement_workflow/ProjectParameters_stage4.json Show resolved Hide resolved
@avdg81 avdg81 marked this pull request as ready for review October 27, 2023 14:50
@avdg81 avdg81 merged commit 7988d63 into master Oct 28, 2023
10 of 11 checks passed
@avdg81 avdg81 deleted the geo/11721-fix-settlement-output branch October 28, 2023 08:06
@rfaasse rfaasse added the GeoMechanics Issues related to the GeoMechanicsApplication label Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avdg81 avdg81 avdg81 approved these changes

@WPK4FEM WPK4FEM Awaiting requested review from WPK4FEM

Assignees
No one assigned
Labels
GeoMechanics Issues related to the GeoMechanicsApplication
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] Fix settlement output for normal load and inactive nodes/excavation
3 participants
@rfaasse @avdg81 @WPK4FEM