search

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
1.03k stars 245 forks source link

Standard Python solvers for the Fluid Dynamics Application #871

Closed jcotela closed 5 years ago

jcotela commented 7 years ago

We need to do a bit of clean-up of the FluidDynamicsApplication python solvers: right now we have a lot of them and abundant duplication. I think this would also be a good opportunity to settle on a common solver class and a uniform way of dealing with processes or, at the very least, have a set of "clean and nice" solvers that work from GiD.

Tasks:

pooyan-dadvand commented 7 years ago

:+1: for adding this for the next release!

jcotela commented 7 years ago

Hi guys, I'm realizing that this is a very ambitious change for a week (the planned date for the fork), so I'm dropping it from the next Release. I'd postpone it to the eventual release 5.3. :(

philbucher commented 6 years ago

may I add #1090 to the list? :)

philbucher commented 6 years ago

and maybe what I did in StructuralMechanics: Having the possibility to define Variables in ProjectParams that are to be added to the ModelPart, see #1113 (see auxiliary_variables_list)

jcotela commented 6 years ago

Hi @philbucher I'll look into it. However, I'd prefer a solution in the line of #869, at least for utilities or processes that will always need the same variables.

philbucher commented 6 years ago

hi @jcotela it is just a suggestion, it might not be relevant for the Fluids.

Just as an example of when I need it: Some elements in the StructuralMechanicsApp can compute results for variables that are not added by the solver by default. Those I can add now through ProjectParams to ask for results in the outputprocess.

jcotela commented 6 years ago

@philbucher I agree that this is needed, also for the fluids. And your solution is the only one that works as the code is now. Still, I think it is the job of the output process to read those variables and take care that they have been added, as this would make the code more modular.

philbucher commented 6 years ago

Now I understood, the output process is treated in the same way! Somehow I haven't seen thos before

But yes, it makes absolute sense (I will revert my changes then once the adding is working)

jcotela commented 6 years ago

Just for future reference, this is the proposed list of solvers:

All current solvers not on the list will be removed in the final stage of the clean-up. Most of them are outdated or duplicates, so no features should be lost.

jcotela commented 6 years ago

Note to self/to-do: default settings for different solvers should have "wrong" values when they must be set from outside (domain_size is an obvious example, which is currently initialized to 2 in most solvers).

philbucher commented 5 years ago

I think it would be nice to have this for the release

philbucher commented 5 years ago

Just to add that it might me worth changing the default mpi-solver from Trilinos-MultiLevel to AMGCL (FYI @ddemidov ) since @AndreasWinterstein found out in his recent scaling-studies that it is much faster

Also then the same solver would be used in serial and MPI

RiccardoRossi commented 5 years ago

+1 on my side (and kudos to @ddemidov)

On Tue, Feb 5, 2019, 10:04 AM Philipp Bucher <notifications@github.com wrote:

Just to add that it might me worth changing the default mpi-solver from Trilinos-MultiLevel to AMGCL (FYI @ddemidov https://github.com/ddemidov ) since @AndreasWinterstein https://github.com/AndreasWinterstein found out in his recent scaling-studies that it is much faster

Also then the same solver would be used in serial and MPI

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/KratosMultiphysics/Kratos/issues/871#issuecomment-460561497, or mute the thread https://github.com/notifications/unsubscribe-auth/AHr7Edu6CAaLbaioMAV6Gtk-oShFiAppks5vKUkGgaJpZM4QARJV .

pooyan-dadvand commented 5 years ago

kudos @ddemidov!

philbucher commented 5 years ago

adding #4390 to the list

philbucher commented 5 years ago

adding #4736 to the list (once #4736 is merged)

philbucher commented 5 years ago

is this really closed? I think there are some things left to do

jcotela commented 5 years ago

@philbucher kind of... @rubenzorrilla and me agreed not to rename the existing solvers for now. I believe that the only solver on that list that is really missing is the trilinos compressible solver, but there is no movement on that front at the moment

philbucher commented 5 years ago

I meant #4390 and #4121 are not implemented yet

jcotela commented 5 years ago

@philbucher I added them to the FluidDynamicsApplication project, I think it will be easier to keep track of them in this way.