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 class for carrying out a single time step #11617

Closed
avdg81 opened this issue Sep 27, 2023 · 5 comments · Fixed by #11649
Closed

[GeoMechanicsApplication] Add a class for carrying out a single time step #11617

avdg81 opened this issue Sep 27, 2023 · 5 comments · Fixed by #11649
Assignees
Labels
Feature GeoMechanics Issues related to the GeoMechanicsApplication

Comments

@avdg81
Copy link
Contributor

avdg81 commented Sep 27, 2023

As a developer I would like to be able to carry out a single time step, given the solver strategy and a list of processes to take into account.

Background

The Kratos team at Deltares would like to integrate time stepping into their custom workflows in a "modular" fashion. By "modular" we mean that we should be able to customize the time stepping by making various compositions of components rather than by modifying a monolithic block that includes all aspects of time stepping. Carrying out the time step at any given time and for the given solver strategy and a list of processes is one of those components.

Acceptance criteria

Given the solver strategy does not converge
When the developer carries out the time step (by calling the TimeStepExecuter<...>::Run(...) member function)
Then the returned time step state should indicate that it did not converge.

Given the solver strategy converges
When the developer carries out the time step (by calling the TimeStepExecuter<...>::Run(...) member function)
Then the returned time step state should indicate that it converged.

Given the solver strategy converges and a list containing a single process
When the developer carries out the time step (by calling the TimeStepExecuter<...>::Run(...) member function)
Then the process's member functions ExecuteInitializeSolutionStep() and ExecuteFinalizeSolutionStep() each must have been called exactly once.

Given the solver strategy converges
When the developer carries out the time step (by calling the TimeStepExecuter<...>::Run(...) member function)
Then the solver strategy's member functions Initialize(), InitializeSolutionStep(), Predict(), SolveSolutionStep(), and FinalizeSolutionStep() each must have been called exactly once.

Given the solver strategy converges in N iterations
When the developer carries out the time step (by calling the TimeStepExecuter<...>::Run(...) member function)
Then the returned time step state should indicate that the convergence took N iterations.

Given the solver strategy converges for time T
When the developer carries out the time step (by calling the TimeStepExecuter<...>::Run(...) member function)
Then the returned time step state should indicate that the corresponding time equals T.

Given the solver strategy does not converge for time T
When the developer carries out the time step (by calling the TimeStepExecuter<...>::Run(...) member function)
Then the returned time step state should indicate that the corresponding time equals T.

@avdg81 avdg81 self-assigned this Sep 27, 2023
@avdg81 avdg81 converted this from a draft issue Sep 27, 2023
@avdg81 avdg81 added Feature GeoMechanics Issues related to the GeoMechanicsApplication labels Sep 27, 2023
@rfaasse
Copy link
Contributor

rfaasse commented Sep 28, 2023

@avdg81 @WPK4FEM These acceptance criteria are very clear and complete! However, I think they could be generalized a bit to not have details on how exactly our tests run in these AC (and it should also work for the more general case):

  1. I would replace 'Given a solver strategy that always/never converges' with Given the solver strategy converges/does not converge (because this should just work with any solver strategy when it converges/does not converge).
  2. For AC 5 I would make it Given a solver strategy converges in N iterations ... Then ... convergence took N iterations
  3. For AC 6, I would change it to Given a solver strategy converged at a time X, ... Then ... time equals X

One last question: Should outputting the results be taken up in this issue, or in another one?

@RiccardoRossi
Copy link
Member

hi, this is an interesting thought, and is also something in which we were thinking in the past.

We were facing some troubles with this kind ideas in two aspects:
1 - substepping
2 - coupling of multiple solvers.

could we maybe have an informal meeting so that you explain me your idea?

@avdg81
Copy link
Contributor Author

avdg81 commented Sep 28, 2023

hi, this is an interesting thought, and is also something in which we were thinking in the past.

We were facing some troubles with this kind ideas in two aspects: 1 - substepping 2 - coupling of multiple solvers.

could we maybe have an informal meeting so that you explain me your idea?

Hello, and thank you for your comment. Yes, we can discuss our ideas informally in an online meeting. In that case I would like to also invite @WPK4FEM (since he is involved in these developments too) and @mcgicjn2. I will contact you later by e-mail to actually plan the meeting.

@mcgicjn2
Copy link
Contributor

Hi, Riccardo,

We can discuss in a meeting, and raise this to core level, if you prefer.

We do have an urgency for this mechanism as it is a bottleneck towards a new product we will release in the coming weeks. So I'd prefer if we get a first implementation into our applications and then migrate to the core, with feedback, improvements and discussion with the wider community.

The time stepping is essentially important to us, as you can imagine, we have several applications where everything happens in the first few timesteps and then the remainder happens over long time periods, e.g. primary consolidation (days) followed by creep (decades) thus our need for a time, and we have the inverse problem, where nothing happens and then everything at once, e.g. slope collapse. Obviously we also have numerical stability issues to also take into account.

@RiccardoRossi
Copy link
Member

fair enough for the local implementaiton.

let's write offline to organize a meeting. my email is [email protected]

@avdg81 avdg81 moved this from 👷 In Progress to 👀 In Review in Kratos Product Backlog Oct 4, 2023
@avdg81 avdg81 linked a pull request Oct 5, 2023 that will close this issue
@avdg81 avdg81 closed this as completed Oct 5, 2023
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in Kratos Product Backlog Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature GeoMechanics Issues related to the GeoMechanicsApplication
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants