Kratos
Kratos copied to clipboard
KratosMultiphysics
[Core][Analysis] Define _AdvanceTime in base analysis
📝 Description
In @KratosMultiphysics/altair we define in the analysis the method _AdvanceTime, which by default calls the method from the solver as it is done right now in the base analysis class. I think unifying interfaces could benefit, reducing code duplication and extending modularity of current analysis
🆕 Changelog
- Define _AdvanceTime in base analysis
With this you're opening the possibility to calculate the time step in the analysis stage, something that now is forbidden, making mandatory to calculate it in the solver.
I'd prefer to keep the way it is right now, but we can discuss it in the @KratosMultiphysics/technical-committee.
As I said, we do like this in @KratosMultiphysics/altair, the idea is to unify interfaces. (and this is just one step, the simplest one I found...)
My thoughts on this :)
We define start_time, end_time in the problem_data section, and time step somewhere in the solver_settings (not in a consistent manner also). Which kind of feels like time marching is a shared responsibility between the AnalysisStage and PythonSolver. Wouldn't it be better to leave this responsibility with one class? It would be harder with the automatic time marching, but is there a way to have it also? Just looking for options :)
We define
start_time,end_timein theproblem_datasection, and time step somewhere in thesolver_settings(not in a consistent manner also). Which kind of feels like time marching is a shared responsibility between theAnalysisStageandPythonSolver. Wouldn't it be better to leave this responsibility with one class? It would be harder with the automatic time marching, but is there a way to have it also? Just looking for options :)
from my point of view, start_time and end_time belong to the continuous problem and the time_step belongs to the discrete problem. So I find cleaner having different classes managing different things. Of course, they have the same units, but are still different.
We define
start_time,end_timein theproblem_datasection, and time step somewhere in thesolver_settings(not in a consistent manner also). Which kind of feels like time marching is a shared responsibility between theAnalysisStageandPythonSolver. Wouldn't it be better to leave this responsibility with one class? It would be harder with the automatic time marching, but is there a way to have it also? Just looking for options :)from my point of view,
start_timeandend_timebelong to the continuous problem and thetime_stepbelongs to the discrete problem. So I find cleaner having different classes managing different things. Of course, they have the same units, but are still different.
Totally agree. Note that for the same physics time marching might be completely different depending on the explicit/implicit nature of the strategy, something that is managed at the solver level.
I agree that the better place for the AdvanceTimeStep function is inside solver. Our problem was that in some cases we want to finish our analysis exactly at the final time given by the user. The end of the simulation is controlled inside the analysis, that is why the simpler solution I saw was to move the AdvanceTime function to the analysis so it can change the delta time provided by the solver so it can be adjusted with respect to the final time.
so, we @KratosMultiphysics/technical-committee agree about this change.
also once we give up with this restriction we shall also add a _Predict() and _SolveSolutionStep() methods defaulting to the current behaviour. We shall comment those functions telling that by default they should call the solver ...
why did you call it _AdvanceTime and not _AdvanceInTime (consistent with the solver)?
You are technically right, but the main point was to unify with our internal analysis implementation (maybe we should have changed that at that time). Do you consider that in this case we should impose consistency?
El vie., 7 abr. 2023 0:04, Philipp Bucher @.***> escribió:
why did you call it _AdvanceTime and not _AdvanceInTime (consistent with the solver)?
— Reply to this email directly, view it on GitHub https://github.com/KratosMultiphysics/Kratos/pull/10217#issuecomment-1499679161, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYQZADURFASK7VP3YK2OPDW744WNANCNFSM6AAAAAAQDKSJEE . You are receiving this because you modified the open/close state.Message ID: @.***>
Given that this is the main interface to Kratos and super easy to make backward compatible, I would say yes, it is worth unifying