Conversation
…into core/adaptive-time
| KratosMultiphysics.Logger.PrintInfo(self.__class__.__name__, f"Did not converge for time = {self.time}.") | ||
|
|
||
| current_step_controller_time = time_begin | ||
| while not step_controller.IsCompleted(current_step_controller_time, is_converged): |
There was a problem hiding this comment.
@KratosMultiphysics/technical-committee do we want to over complicate the base AnalysisStage like this? I believe that the feature is useful but...
There was a problem hiding this comment.
I agree, this is lots of specialized code, IMO the base AnalysisStage is not the place for this
| "convergence_factor" : 1.5, | ||
| "delta_t_init" : 1.0, | ||
| "delta_t_min" : 1.0, | ||
| "delta_t_max" : 1.0, |
There was a problem hiding this comment.
delta_time for example would be clearer. What is divergence factor?
RiccardoRossi
left a comment
There was a problem hiding this comment.
Blocking since the design needs to be reviewed and discussed by @KratosMultiphysics/technical-committee .
@sunethwarna can you schedule this for one of the upcoming meetings?
|
moved all the implementatiosn from core to the StructApp under a new derrived analysis. |
Looks like is missing something |
...StructuralMechanicsApplication/python_scripts/structural_mechanics_load_stepping_analysis.py
Outdated
Show resolved
Hide resolved
...StructuralMechanicsApplication/python_scripts/structural_mechanics_load_stepping_analysis.py
Outdated
Show resolved
Hide resolved
RiccardoRossi
left a comment
There was a problem hiding this comment.
nice PR. I have some comments (the most important is that the Predict needs to be treated as in other analysis stages), but overall i like the idea
|
|
||
| # find the appropriate step controller for the given time frame | ||
| step_controller = None | ||
| for interval_utility, step_controller in reversed(list_of_step_controllers): |
There was a problem hiding this comment.
why don't you combine the step controllers in one? my guess is that it would be cleaner not to have to handle a list of step controllers
There was a problem hiding this comment.
The reasoning is, the users wants to use different step controllers in different time ranges. This makes it possible. If you combine it to one, that granularity is gone as far as i understand.
There was a problem hiding this comment.
What @RiccardoRossi probably means is to create smth like a CompoundStepController that applies different controllers on different time ranges.
The effect and flexibility is essentially the same, but the complexity is hidden in CompoundStepController instead of AnalysisStage. I think that's a valid point.
There was a problem hiding this comment.
By the way, why is the list reversed?
|
|
||
| IsCompleted(current_time: float, is_converged: bool) -> bool | ||
| Check if the stepping process has been completed. | ||
| """ |
There was a problem hiding this comment.
Please convert all docstrings to doxygen style.
| default_parameters = Kratos.Parameters("""{ | ||
| "type": "default_step_controller" | ||
| }""") |
There was a problem hiding this comment.
For future compatibility with JSON schemas, it'd be great if you could pull default params to a separate function (smth like GetDefaultParameters).
| Attributes: | ||
| __divergence_factor: Factor by which the time step is reduced after a failed attempt. | ||
| __convergence_factor: Factor by which the time step is increased after successful attempts. | ||
| __delta_t_init: Initial time step size. | ||
| __delta_t_min: Minimum allowed time step size. | ||
| __delta_t_max: Maximum allowed time step size. | ||
| __max_number_of_sub_steps: Maximum number of sub-steps allowed. | ||
| __number_of_successful_attempts_for_increment: Number of successful steps required to increase the time step. | ||
| __number_of_failed_attempts_for_termination: Number of failed steps allowed before termination. | ||
| __sub_stepping_iteration: Counter for the current sub-step iteration. | ||
| __success_full_attempts_count: Counter for successful attempts. | ||
| __failed_attempts_count: Counter for failed attempts. | ||
| __delta_time: Current time step size. | ||
| __time_begin: Start time of the stepping process. | ||
| __time_end: End time of the stepping process. |
There was a problem hiding this comment.
These are internal variables and must not be in the docs.
| @abstractmethod | ||
| def Initialize(self, time_begin: float, time_end: float) -> None: | ||
| """ | ||
| Initializes the step controller with the specified start and end times for current time step from the solver. | ||
|
|
||
| Args: | ||
| time_begin (float): The starting time of the current time step from the solver. | ||
| time_end (float): The ending time of the current time step from the solver. | ||
|
|
||
| Returns: | ||
| None | ||
| """ | ||
| pass |
There was a problem hiding this comment.
Why have a separate Initialize function? I'd rather construct a fresh step controller for every time step that gets initialized on construction and destroyed at the end of the time step.
This would help reduce confusion with the initialize/finalize chaos we have with solvers/processes/elements/schemes/strategies. I don't think we need yet another layer of init/fin ops to keep track of.
| if self.__success_full_attempts_count > self.__number_of_successful_attempts_for_increment: | ||
| # now we increment the step | ||
| self.__delta_time = min(self.__convergence_factor * self.__delta_time, self.__delta_t_max) | ||
| Kratos.Logger.PrintInfo(self.__class__.__name__, f"Incrementing the delta time to {self.__delta_time}") |
There was a problem hiding this comment.
Please hide printing status info behind a verbosity check. We don't always need to pollute our output with this.
|
|
||
| # find the appropriate step controller for the given time frame | ||
| step_controller = None | ||
| for interval_utility, step_controller in reversed(list_of_step_controllers): |
There was a problem hiding this comment.
What @RiccardoRossi probably means is to create smth like a CompoundStepController that applies different controllers on different time ranges.
The effect and flexibility is essentially the same, but the complexity is hidden in CompoundStepController instead of AnalysisStage. I think that's a valid point.
|
|
||
| # find the appropriate step controller for the given time frame | ||
| step_controller = None | ||
| for interval_utility, step_controller in reversed(list_of_step_controllers): |
There was a problem hiding this comment.
By the way, why is the list reversed?
| def __get_serializer(): | ||
| return Kratos.FileSerializer("serialization", Kratos.SerializerTraceType.SERIALIZER_NO_TRACE, True) |
There was a problem hiding this comment.
ugh
I'd like to see an in-memory solution to this. Please open a new PR or issue while we still have this in mind.
| parameters = Kratos.Parameters(parameter_file.read()) | ||
|
|
||
| model = Kratos.Model() | ||
| simulation = StructuralMechanicsAnalysis(model, parameters) |
There was a problem hiding this comment.
I thought you defined the new analysis type for a reason.
📝 Description
** NO BEHAVIOR CHANGE IS THERE **
This PR adds load stepping controlling mechanism to try converging non-linear non-converged solutions by load stepping, and changing the time step. This is designed to work in two ways.
tvariable to this to work.delta_t, and increasing mass effect to make the non-converging time steps to converge.This requires all the entities used in the simulations does not hold any private member variables storing the state such as
DISPLACEMENTSorVELOCITYsince this requires resetting them. Currently this does not work with following element types:CrBeamElement3D2NCrBeamElement2D2NI will wait until #13432 merged, so that there won't be conflicts from this branch to that.
🆕 Changelog
StepControllerclassDefaultStepControllerto have the existing behavior with no changes at allGeometricStepControllerfollowing the abaqus implementation of load stepping.