[GeoMechanicsApplication] Fix bug in time step mechanism C++ workflow by rfaasse · Pull Request #14184 · KratosMultiphysics/Kratos

rfaasse · 2026-02-06T08:59:02Z

📝 Description
In the time stepping scheme in the C++ workflow, avoiding a small step was not done for the very first time-step, which resulted in an error for cases where the first increment of a stage would result in a very small subsequent step to get to the end-time.

This PR adds the same upscaling functionality that was there for subsequent steps also in the constructor, to check the very first step of a stage.

Changelog:

The first time step is now checked for avoiding a small last step
An integration test for the C++ workflow is added
Unit tests are added

…TimeIncrementor interface

…d unit tests accordingly. Also made the moments of checking the delta time consistent with the python workflow.

…ent function.

…n but also added it to the constructor.

rfaasse · 2026-02-06T08:59:31Z

applications/GeoMechanicsApplication/custom_workflows/adaptive_time_incrementor.cpp

      mReductionFactor(ReductionFactor),
      mIncreaseFactor(IncreaseFactor),
-      mUserMinDeltaTime(std::move(UserMinDeltaTime)),
+      mUserMinDeltaTime(UserMinDeltaTime),


Clang-tidy mentioned the move of a std::optional was not necessary

rfaasse · 2026-02-06T09:09:55Z

applications/GeoMechanicsApplication/custom_workflows/adaptive_time_incrementor.cpp

    KRATOS_ERROR_IF(MaxTimeStepFactor < 1.0)
        << "Max_delta_time_factor must be greater than or equal to 1, but got " << MaxTimeStepFactor;
+
+    CorrectDeltaTimeToEndTime(StartTime);


This is the actual fix, we need to check at construction to make sure the first increment is correct

rfaasse · 2026-02-06T09:16:32Z

applications/GeoMechanicsApplication/custom_workflows/adaptive_time_incrementor.cpp

+    return mUserMinDeltaTime.value_or(default_min_delta_time);
+}
+
+void AdaptiveTimeIncrementor::CorrectDeltaTimeToEndTime(double StartOfIncrement)


I'm struggling with the name of this function. What it does is if the deltatime would result in either a very small next step (difference smaller than min delta time) or if it would result in going beyond the end time, it corrects it to be the end time. If you have any suggestions for a better name, let me know

CorrectDeltaTimeToReachEndTime or AdaptDeltaTimeToAvoidSmallNextIncrement

These are both better than the current name, but they don't capture the 'preventing overshooting the end time' part of the function, so I'm still wondering if there's an even better name somewhere

I agree that it's very hard to put the exact checks in words without ending up with extremely long function names. What about AdaptDeltaTimeIfNeeded? It doesn't spell out when it's needed, just that the time increment may or may not be modified after doing some checks.

I think that's a good idea (since I don't see a way to capture both reasons in the function name). I did add a comment within the function to explain what it's doing. Hope that helps

rfaasse · 2026-02-06T09:17:13Z

applications/GeoMechanicsApplication/tests/cpp_tests/custom_workflows/test_time_incrementor.cpp

    auto previous_state              = TimeStepEndState{};
    previous_state.convergence_state = TimeStepEndState::ConvergenceState::converged;

-    time_incrementor.PostTimeStepExecution(previous_state);


Removing this line makes sure we don't do the correction that's in PostTimeStepExecution, to make sure a first step is also corrected

WPK4FEM · 2026-02-06T09:36:30Z

applications/GeoMechanicsApplication/tests/test_GeoMechanicsApplication.py

+                        KratosGeoMechanicsAvoidSmallTimeStepPythonRoute,
+                        KratosGeoMechanicsAvoidSmallTimeStepCppRoute,


I find the SmallEndStep more descriptive than SmallTimeStep

avdg81

Dear @rfaasse and @WPK4FEM,
Thanks a lot for preparing this bug fix. The changes were clear, and are well-covered by the new unit tests and integration test. I only have a few minor comments, none of them being blocking. I feel this PR is almost ready to go 👍

avdg81 · 2026-02-06T13:03:25Z

applications/GeoMechanicsApplication/custom_workflows/adaptive_time_incrementor.cpp

+    CheckMinimumDeltaTime(mDeltaTime);
+}
+
+void AdaptiveTimeIncrementor::CheckMinimumDeltaTime(double DeltaTime) const


As far as I can see, this member function is always called given the data member mDeltaTime. Since it has access to this data directly, we don't really need to pass it through the argument list. Since it's a private member function, client code won't be able to call it directly. Would it be cleaner to have an empty argument list and fetch the required data directly inside the function body?

Agreed, done!

avdg81 · 2026-02-06T13:10:48Z

applications/GeoMechanicsApplication/custom_workflows/adaptive_time_incrementor.cpp

+    return mUserMinDeltaTime.value_or(default_min_delta_time);
+}
+
+void AdaptiveTimeIncrementor::CorrectDeltaTimeToEndTime(double StartOfIncrement)


I agree that it's very hard to put the exact checks in words without ending up with extremely long function names. What about AdaptDeltaTimeIfNeeded? It doesn't spell out when it's needed, just that the time increment may or may not be modified after doing some checks.

avdg81 · 2026-02-06T13:19:37Z

applications/GeoMechanicsApplication/tests/cpp_tests/custom_workflows/test_time_incrementor.cpp

    auto previous_state              = TimeStepEndState{};
    previous_state.convergence_state = TimeStepEndState::ConvergenceState::non_converged;
-    previous_state.time              = 8.0;
+    previous_state.time              = 0.0;


Good catch!

avdg81

Thanks a lot for processing the review suggestions. To me, this PR looks good to go.

avdg81 · 2026-02-06T16:35:52Z

applications/GeoMechanicsApplication/custom_workflows/adaptive_time_incrementor.cpp

+    // If the delta time would result in
+    //  a. the next step exceeding the end time or
+    //  b. having too small a remaining time step
+    // then the delta time is chosen such that the next step ends exactly at the end time.


Very clear, thank you.

markelov208

Hi Richard, thank you for fixing this bug and adding unit and integration tests. The PR looks very good for me.

rfaasse added 16 commits February 5, 2026 11:50

Added integration test for the Cpp workflow (re-use the python version)

f89b634

Added previous time as argument for the GetIncrement function in the …

3a6ca68

…TimeIncrementor interface

Changed the moment of upscaling to avoid a small end step and adjuste…

3e0d7d4

…d unit tests accordingly. Also made the moments of checking the delta time consistent with the python workflow.

Removed redundant line

3e63697

Added unit tests for covering minimum timestep check in the GetIncrem…

86e97b9

…ent function.

Simplified and refactored the adaptive time incrementor functions.

d188568

Moved the check to avoid small steps back to the PostTimeStepExecutio…

6d5b6bc

…n but also added it to the constructor.

Removed previous time argument from get increment method

d4637f0

Removed redundant variable

1770b52

Made some functions private, and did reformatting

6ba5f21

Moved private functions to bottom of source file

71e6b39

Extracted function for correcting delta time

dbf62e7

Removed redundant std::move

2ba0923

Minor clean-up

ad08f12

Fixed incorrect comments

2c0435a

Formatting

0b725e5

rfaasse assigned rfaasse and WPK4FEM Feb 6, 2026

rfaasse added Bugfix GeoMechanics Issues related to the GeoMechanicsApplication labels Feb 6, 2026

rfaasse linked an issue Feb 6, 2026 that may be closed by this pull request

[GeoMechanicsApplication] Bug in time step mechanism C++ workflow #14153

Closed

rfaasse changed the title ~~[GeoMechanicsApplication] Fix bug in time step mechanism c++ workflow~~ [GeoMechanicsApplication] Fix bug in time step mechanism C++ workflow Feb 6, 2026

rfaasse commented Feb 6, 2026

View reviewed changes

WPK4FEM reviewed Feb 6, 2026

View reviewed changes

rfaasse added 2 commits February 6, 2026 10:43

Renamed python test

e78059f

Removed unused TimeStepEndState

46d5f17

rfaasse requested review from avdg81 and markelov208 February 6, 2026 09:45

rfaasse marked this pull request as ready for review February 6, 2026 09:45

rfaasse requested a review from a team as a code owner February 6, 2026 09:45

Formatting

b01172c

avdg81 reviewed Feb 6, 2026

View reviewed changes

Processed first review comments

72dc41b

avdg81 approved these changes Feb 6, 2026

View reviewed changes

markelov208 reviewed Feb 9, 2026

View reviewed changes

rfaasse merged commit 7882485 into master Feb 9, 2026
10 checks passed

rfaasse deleted the geo/fix-bug-in-time-step-mechanism-c-workflow branch February 9, 2026 15:06

		KratosGeoMechanicsAvoidSmallTimeStepPythonRoute,
		KratosGeoMechanicsAvoidSmallTimeStepCppRoute,

Conversation

rfaasse commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WPK4FEM Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avdg81 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avdg81 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markelov208 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rfaasse commented Feb 6, 2026 •

edited

Loading

WPK4FEM Feb 6, 2026 •

edited

Loading