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

[GeoMechanicsApplication] Geo/time step loop #11677

Merged
merged 26 commits into from
Oct 16, 2023
Merged

[GeoMechanicsApplication] Geo/time step loop #11677

merged 26 commits into from
Oct 16, 2023

Conversation

WPK4FEM
Copy link
Contributor

@WPK4FEM WPK4FEM commented Oct 12, 2023

TIme loop executor that performs the stepping loop and for each step the cycle loop using TimeStepExecutor and StrategyWrapper..

Contains:

  • Unit tests for all developed components
  • CloneTimeStep for claiming variable space at start of step
  • RestorePosistionsAndDOFVectorToStartOfStep for enabeling the cycling mechanism
  • SaveTotalDsplacementFieldAtStartOfStage and AccumulateTotalDisplacementField for keeping track of displacement accumulation over stages
  • OutputProces to enable output at the end of each step

WPK4FEM and others added 16 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.
@WPK4FEM
Added method and tests for RestorePositionsAndDOFVectorToStartOfStep …
916242f 
…for restoring to the initial situation of a step to enable cycling of the step.
@WPK4FEM
SaveTotalDisplacementFieldAtStartOfStage and AccumulateTotalDisplacem…
9753302 
…entField implemented and unit tests provided.
@WPK4FEM
OutputProcess implementation and unit test added to time_loop_executor.
9ba7a26
@WPK4FEM WPK4FEM added the GeoMechanics Issues related to the GeoMechanicsApplication label Oct 12, 2023
@WPK4FEM WPK4FEM self-assigned this Oct 12, 2023
rfaasse
rfaasse requested changes Oct 12, 2023
View reviewed changes
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.

Very nice readable and clean code (and of course well-tested)!

I think there is the possibility for some more clean-up of the tests and there are some discussion points about the structure.

applications/GeoMechanicsApplication/CMakeLists.txt Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/custom_workflow_factory.cpp Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.h Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.hpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.hpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/stub_time_loop_executor.h Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.h Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Outdated Show resolved Hide resolved
@WPK4FEM WPK4FEM linked an issue Oct 12, 2023 that may be closed by this pull request
[GeoMechanicsApplication] Add a class for running a time loop #11620
Closed
rfaasse
rfaasse reviewed Oct 13, 2023
View reviewed changes
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor_interface.h

class Process;

class TimeLoopExecutorInterface{
Copy link
Contributor

Choose a reason for hiding this comment

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

@avdg81 We discussed we didn't want Interface in the name, but @WPK4FEM and I could not think of a suitable pair of names for the interface + implementation of the time loop executor. If you have a suggestion, please let us know!

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I don't have a better suggestion either. Let's keep the class names as they are now.

@WPK4FEM WPK4FEM requested a review from rfaasse October 13, 2023 09:27
avdg81
avdg81 reviewed Oct 13, 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.

Clear and clean code. Good job! I have several comments, most of which are very minor.

applications/GeoMechanicsApplication/custom_workflows/dgeosettlement.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.hpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.hpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.hpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor_interface.h

class Process;

class TimeLoopExecutorInterface{
Copy link
Contributor

Choose a reason for hiding this comment

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

No, I don't have a better suggestion either. Let's keep the class names as they are now.

applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Outdated Show resolved Hide resolved
applications/GeoMechanicsApplication/tests/cpp_tests/test_time_loop_executor.cpp Show resolved Hide resolved
@WPK4FEM WPK4FEM requested a review from avdg81 October 16, 2023 06:30
rfaasse
rfaasse approved these changes Oct 16, 2023
View reviewed changes
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.

Thanks for implementing the changes and the collaboration on this one! It turned out nicely, so from my side, it's ready to be merged

@WPK4FEM WPK4FEM merged commit f71b2f6 into master Oct 16, 2023
9 of 11 checks passed
@WPK4FEM WPK4FEM deleted the geo/time-step-loop branch October 16, 2023 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfaasse rfaasse rfaasse approved these changes

@mcgicjn2 mcgicjn2 Awaiting requested review from mcgicjn2

@avdg81 avdg81 Awaiting requested review from avdg81

Assignees

@WPK4FEM WPK4FEM

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

Successfully merging this pull request may close these issues.

[GeoMechanicsApplication] Add a class for running a time loop
3 participants
@WPK4FEM @avdg81 @rfaasse