KratosMultiphysics / Kratos

Kratos Multiphysics (A.K.A Kratos) is a framework for building parallel multi-disciplinary simulation software. Modularity, extensibility and HPC are the main objectives. Kratos has BSD license and is written in C++ with extensive Python interface.
https://kratosmultiphysics.github.io/Kratos/
Other
1k stars 243 forks source link

[FluidDyn] reform_dofs_at_each_step is set to True by default in the vms solver. #5229

Closed marcnunezc closed 5 years ago

marcnunezc commented 5 years ago

@KratosMultiphysics/fluid-dynamics

When running a 2D transient navier-stokes case with the vms solver, and checking the time spent in each function of the analysis stage, a lot of time was spent in IntializeSolutionStep() (around 30%)

After checking it with @RiccardoRossi, he saw that in the default settings the option "reform_dofs_at_each_step" was set to true:

https://github.com/KratosMultiphysics/Kratos/blob/f95022ba0a56f30de67f77bdd399f7ce8f1a65cb/applications/FluidDynamicsApplication/python_scripts/navier_stokes_solver_vmsmonolithic.py#L164

Is there a reason for this option to be enabled by default?

It is also set to true by default in the compressible solver: https://github.com/KratosMultiphysics/Kratos/blob/f95022ba0a56f30de67f77bdd399f7ce8f1a65cb/applications/FluidDynamicsApplication/python_scripts/navier_stokes_compressible_solver.py#L30

When setting it to false the time spent in InitializeSolutionStep() is much lower.

rubenzorrilla commented 5 years ago

If I'm not wrong, in the past we decided (I think that @RiccardoRossi was also involved in the discussion) to keep the true as default behavior because this would avoid errors if there's any internal operation that requires such reform_dofs_at_each_step but the user doesn't know about it. We decided to assume that if it is not needed and the user is aware of this, is the user the one that should change it.

philbucher commented 5 years ago

What could be an operation that requires this? I cannot think of anything except constraints

rubenzorrilla commented 5 years ago

What comes to my mind is any formulation that changes the amount of DOFs on the flight or remeshing.

jcotela commented 5 years ago

As @rubenzorrilla said, the argument for setting it to true by default is robustness. Remeshing (either adaptivity or from mesh deformation/FSI) is the usual case that would break otherwise.

Something that might make sense is set it to always set it to False on the json produced by the GiD Fluid problemtype, since I do not think that we can enable any of the problematic features from the interface. No idea if it is feasible, though.

philbucher commented 5 years ago

Why does mesh-deformation change the number of DOFs?

jcotela commented 5 years ago

@philbucher I used the wrong word there... What I should have said is that some of the quality improvement tools you can use when your mesh is highly deformed can change the number of nodes (in 3D some of the mesh reconnection solutions we have available involve creating new nodes).

RiccardoRossi commented 5 years ago

i didn t remember this decision, although i can understand the rationale

is that so also for the fractional step solver??

i would agree about the proposal of @jcotela

On Fri, Jul 12, 2019, 6:21 PM jcotela notifications@github.com wrote:

@philbucher https://github.com/philbucher I used the wrong word there... What I should have said is that some of the quality improvement tools you can use when your mesh is highly deformed can change the number of nodes (in 3D some of the mesh reconnection solutions we have available involve creating new nodes).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/KratosMultiphysics/Kratos/issues/5229?email_source=notifications&email_token=AB5PWENZHVCXOGKOQS35ESLP7CVRDA5CNFSM4ICK7W72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ2HBHI#issuecomment-510947485, or mute the thread https://github.com/notifications/unsubscribe-auth/AB5PWEI36N2C2NGLE4BH3J3P7CVRDANCNFSM4ICK7W7Q .

philbucher commented 5 years ago

I think that this option is only needed for (as of now) non-standard cases like remeshing, where (again my opinion) it is fairly obvious that it is required

Of course this is the safer option, but at least for standard cases I would set it to false, hence I agree with the proposal of @jcotela

rubenzorrilla commented 5 years ago

I also agree with what @jcotela proposes. It shouldn't be difficult to hard-code this in the GUI writting script.

rubenzorrilla commented 5 years ago

Already working in the fluid and FSI GUI's.