Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General Discussion: Output-Process #3766

Closed
philbucher opened this issue Jan 6, 2019 · 30 comments
Closed

General Discussion: Output-Process #3766

philbucher opened this issue Jan 6, 2019 · 30 comments
Assignees

Comments

@philbucher
Copy link
Member

I am opening this general discussion abt the output-processes (e.g. for GiD).

Right now we have multiple ways for writing output, e.g. for gid, paraview. Those are implemented as Process but also have an extended interface: IsOutputStep and PrintOutput
This (I assume) is because the base-process has the methods ExecuteBeforeOutputStep and ExecuteAfterOutputStep => the non-standard functions are then called inbetween those functions.

On the other hand there are also other processes that print their output in ExecuteFinalizeSolutionStep

The AnalysisStage deals with both, see here

I think it would be nice to unify this situation, which would also make it easier to write a wrapper for switching the output without effort. Plus the current situation is confusing for new users I think.

  • Either we print the output in ExecuteFinalizeSolutionStep, but then the functions ExecuteBeforeOutputStep and ExecuteAfterOutputStep are not really needed/used. Are they used somewhere?
  • Or we print the Output in PrintOutput, then it would probably make sense to have a new baseclass (OutputProcess) which derives from Process. In there we could also implement IsOutputStep in a general way

I don't have a strong opinion on either of those, so what do others think?

@KratosMultiphysics/technical-committee @KratosMultiphysics/interface-committee @KratosMultiphysics/fluid-dynamics @KratosMultiphysics/structural-mechanics

@loumalouomega
Copy link
Member

FYI I started the wrapper process:

https://github.com/KratosMultiphysics/Kratos/tree/core/post-process-wrapper

I think it needs some more functionaries and some more thinking

@ddiezrod
Copy link
Contributor

ddiezrod commented Jan 7, 2019

@philbucher We use the ExecuteBeforeOutputStep and ExecuteAfterOutputStep to do some tricky dirty things in InspireCast. I think it is reasonable to keep them.

@adityaghantasala
Copy link
Member

I am more in favor of the second option.

@philbucher
Copy link
Member Author

FYI I started the wrapper process:

https://github.com/KratosMultiphysics/Kratos/tree/core/post-process-wrapper

I think it needs some more functionaries and some more thinking

cool!
I think we should do this once we agreed here on how to do the output-processes

@philbucher
Copy link
Member Author

I think I did not mention @msandre and @sunethwarna

@maceligueta
Copy link
Member

I just want to add that the current output process (at least the one for GiD) is intended for a single ModelPart (the constructor gets this ModelPart as an argument). For coupled problems, the output process might need more than one ModelPart (2, 3 or more depending on the application). This should not affect the structure of the AnalysisStage in principle, but should be taken into account when re-writing the output process. I see that the branch by @loumalouomega goes in this direction by getting the full Model, which is great for couplings.

@adityaghantasala
Copy link
Member

adityaghantasala commented Jan 8, 2019

Output process as @loumalouomega did is fine I think. that it is it is inheriting from Process and adding IsOutputStep and PrintOutput function.

@loumalouomega but the way it is done in the init function of postprocess_output_process , that is converting the parameters into either vtk or gid or hdf5 , will require to be standardized. I mean we can have a set of parameters which are common to all of them and have only specifics in the "settings" dict. something like this :

    "output_configuration"             : {
        "type"    :   "vtk/hdf5/gid",
        "model_part_names" : "MainModelPart.SubModelPartOne",
       "file_mode" : "binary/ascii",
        "file_label"          : "time",
        "output_control_type" : "step/time",
        "output_frequency"    : 2,
        "nodal_solution_step_data_variables"       : ["VELOCITY","PRESSURE"],
        "nodal_data_value_variables"                   : ["VORTICITY"],
        "nodal_flags"                                           :[],
        "elemental_solution_step_data_variables"       : ["VELOCITY","PRESSURE"],
        "elemental_data_value_variables"                   : ["VORTICITY"],
        "elemental_flags"                                           :[],
        "extra_settings":{
            "body_output"         : true,
            "node_output"         : false,
            "skin_output"         : false,
            "plane_output"       : []
        }
    }

once this is decided upon this structure can be adapted as much as possible in the individual IO process to make it easy and reduce maintenance efforts later on.

@philbucher
Copy link
Member Author

I agree @adityaghantasala
I first wanted to discuss here if it is at all necessary to have a special OutputProcess or not

Then in a next step I would discuss the interface, otherwise the discussions get messed up

@sunethwarna
Copy link
Member

I also like the common interface and @philbucher 2nd option for hdf5 output.

@jrubiogonzalez
Copy link
Member

Hi,

Currently we are doing things in a slightly different way, and I would like to add our view:

  1. Our Output manager is not a process we decided to separate it completely from processes, and trying to keep it as simple as possible.

  2. I think when and what to plot should also be in separated layers, the output frequency is something that can be very application specific (every 1% filled, every 10s, every 10% load increment, 1 out of 3 ). But the way to define what to print is quite common for all problems and applications.

  3. The ExecuteBeforeOutput and ExecuteAfterOutput should be maintained. We are making a heavy use of them.

  4. I agree with @maceligueta that allowing to plot different modelparts is a must.

  5. Out output definition looks like this. Mostly there is one entry per result, and it must be provided with where the information is stored, where in your model must be plot, units, and what we have called a modifier that is an extra object to make some further decissions.

           ` "output_type": "h3d",
             "settings": {
                 "h3d_settings": {
                     "mesh_transformation": {
                         "mesh_units_in": "m",
                         "mesh_units_out": "mm"
                     },
                     "components_modelparts": [
                         ["ingate_103", 103 ],
                         [ "mold_107",  107 ],
                         [ "mainPart_99", 99] ],
                     "simulation_buffer": 4,
                     "creating_date": "2018.12.20",
                     "model_name": "Filling",
                     "subcase": "Filling",
                     "subcase_id": 1,
                     "write_volume_and_condition_set": true,
                     "results": [
                         {
                             "variable": "KratosMultiphysics.TEMPERATURE",
                             "database": "NodalHistoricalDatabase",
                             "modifier": "SkipAir",
                             "units_in": "degK",
                             "units_out": "degC",
                             "submodel_part": "MainModelPart"
                         },
                         {
                             "variable": "KratosMultiphysics.VELOCITY",
                             "database": "NodalHistoricalDatabase",
                             "modifier": "SkipAir",
                             "units_in": "m/s",
                             "units_out": "m/s",
                             "submodel_part": "submodelpart_liquid"
                         },
                         {
                             "variable": "KratosMultiphysics.DISTANCE",
                             "database": "NodalHistoricalDatabase",
                             "modifier": "PlotEveryStep",
                             "units_in": "m",
                             "units_out": "m",
                             "submodel_part": "submodelpart_liquid"
                         },
                         {
                             "variable": "KratosMultiphysics.DISPLACEMENT",
                             "database": "NodalNonHistoricalDatabase",
                             "modifier": "PlotEveryStep",
                             "units_in": "m",
                             "units_out": "m",
                             "submodel_part": "MainModelPart"
                         },
                         {
                             "variable": "KratosMultiphysics.FILLTIME",
                             "database": "NodalNonHistoricalDatabase",
                             "modifier": "PlotLastStep",
                             "units_in": "sec",
                             "units_out": "sec",
                             "submodel_part": "submodelpart_liquid"
                         }`
    

@adityaghantasala
Copy link
Member

@jrubiogonzalez that is a very comprehensive set of settings. I like the idea of possibility to have different output frequencies for different variables. If it possible to adapt this idea in other IOs, I would definitely support this.

@jrubiogonzalez I am curious about how some things work with the settings your posted. So In the JSON input you posted, there is a sub-modelpart specified for each variable. Then does it mean there is one output file for each of those sub-modelparts ?

About the units, I would rather leave out the units from the general settings, this is because, usually the input (mesh, boundary conds, material params) defines the output units of every variable. And in many post processors, there is usually the possibility to calculate a transformation or apply filters to do so. So I would leave this operation (and corresponding JSON settings) from the output process.

@adityaghantasala
Copy link
Member

@maceligueta about the possibility to write multiple modelparts, Is it ok for you to have multiple output_processes defined one for each of the modelparts that is to be output ?
This way one can also write different variables at different frequencies and so on.

@sunethwarna
Copy link
Member

@jrubiogonzalez I am not sure having different frequencies for different variables is support ted by paraview HDF5, as per my knowledge, one needs to have different hdf5 file groups per frequency which will end up having large number of files in a single file group, and with multiple file groups (to visualize in paraview).

What I am thinking is, to have general interface as @adityaghantasala pointed, and under the custom settings adding the specific settings such as different frequencies for different variables. Will this be an option?

ping @msandre

@maceligueta
Copy link
Member

@adityaghantasala , for different frequencies of output, I would go for independent processes. However, when the printing frequency is the same across models I am very interested in printing a single file for the coupled problem. At least in GiD it is quite annoying to merge results for every time step. If they are merged already, life is much easier, as you can switch to postprocess and results are opened automatically.
Please note that I am not referring to SubModelParts but to actual ModelParts with different sets of nodal variables.

@adityaghantasala
Copy link
Member

@maceligueta I see your point. Fortunately this is not a problem for Vtk and hdf5 with vtk(@sunethwarna please correct me if I am wrong) outputs.

@msandre
Copy link
Member

msandre commented Jan 31, 2019

It looks like the second option is preferred. I don't think the json details need to be decided here.

@roigcarlo
Copy link
Member

roigcarlo commented Feb 27, 2019

At the @KratosMultiphysics/technical-committee we also think the second solution is preferred.

@pooyan-dadvand
Copy link
Member

  1. Our Output manager is not a process we decided to separate it completely from processes, and trying to keep it as simple as possible.

@jrubiogonzalez would you please explain more what you mean about a simple interface

@philbucher
Copy link
Member Author

what @jrubiogonzalez says makes sense to me

in fact this is pretty much what @adityaghantasala and I did when we wrote the vtk-output recently

@pooyan-dadvand
Copy link
Member

@KratosMultiphysics/technical-committee after the current feedback, thinks that separating the output and process in different hierarchy of classes would be a better option.

So it would be very important to check if all of the output formats can be fitted in these three methods.
@KratosMultiphysics/interface-committee would you please follow this discussion and check the compatibility of this design with @KratosMultiphysics/design-committee ?

@philbucher
Copy link
Member Author

We discussed this in the @KratosMultiphysics/design-committee together with @jcotela and we think that the three methods are NOT enough for all output-formats

We therefore suggest to derive an OuputProcess from Process and add the methods PrintOutput and IsOutputStep

more reasons:

  • the (output-)processes are already well integrated and nicely working in the Analysis-Stage
    => this also includes the automatic construction. If we go for a separate class, then we need another "Factory"-mechanism
  • they provide a large flexibility for users to do many things, i.e. to accustom many different tasks

@miguelmaso
Copy link
Contributor

GiD output is printing the mesh on ExecuteBeforeSolutionLoop when SingleFile flag is specified. In case MultipleFiles flag is specified, an output is printed.

Could we consider the possibility of calling ExecuteBeforeOutputStep and ExecuteAfterOutputStep before and after calling ExecuteBeforeSolutionLoop?

Another possibility would be to add something like InitializeResults on the derived OutputProcess.

@jginternational
Copy link
Member

Is this debate still open?

@philbucher
Copy link
Member Author

yes,

@pooyan-dadvand
Copy link
Member

We discussed this in the @KratosMultiphysics/design-committee together with @jcotela and we think that the three methods are NOT enough for all output-formats

We therefore suggest to derive an OuputProcess from Process and add the methods PrintOutput and IsOutputStep

more reasons:

  • the (output-)processes are already well integrated and nicely working in the Analysis-Stage
    => this also includes the automatic construction. If we go for a separate class, then we need another "Factory"-mechanism
  • they provide a large flexibility for users to do many things, i.e. to accustom many different tasks

@KratosMultiphysics/technical-committee agrees with this design. Thanks for the suggestion.

Is there any volunteer for implementing it?

@ddiezrod
Copy link
Contributor

@philbucher As I promised here #7085 : P

I implemented something recently in @KratosMultiphysics/altair side that might be interesting here too. I created an altair_output_process that wraps two other objects: a controller and an io_process. The controller is always in charge of saying when we print, so it is called in IsOutputStep. The io_process controls what to print (VELOCITY, PRESSURE, ...) and how (vtk, h3d, ...) so it is called in PrintOutptut and in all Execute* methods.

They are both created inside the process using a factory (with python module and kratos_module) so they are completely interchangeable, so any controller can go with any io_process.

The objective in mid-term would be to be able to tell the io_process to choose vtk or gid as output format just changing one parameter. This requires to unify interfaces, which its quite a difficult job (maybe impossible to do it perfectly)

@adityaghantasala
Copy link
Member

@ddiezrod I like the idea. But I am worried about the overhead with all the associated factories and associated files.

@pooyan-dadvand
Copy link
Member

IMO, what we have agreed in this discussion is not very practical and introduces a lot of code duplication because if I want to change the control of one format I should derive/implement another process for each format I want to use. For example, writing the results in each 5% of the fill would need to implement a process for vtk format, another for HDF5, and another for h3d. Writing some results only at the end is another example of duplications for all formats.

So let's take a look again at the design. We have 4 main ingredients here:

  1. What we want to print: This includes which variable (pressure, velocity), which entity (node, element, gauss point)
  2. When to print: Each time step, every 5% fill, activated by censor, etc.
  3. Which format: Gid, VTK, H3D, HDF5, etc
  4. Postprocessing: Like changing the units, avoid writing results in the not calculated area, etc. These are optional but useful

In our current design, we are mixing the 1,2, and 3 in big generic output processes per each format which causes mentioned problems. And we pass the 4 to the normal processes where it is not very well fitted. This also obliges us to store all the post-processed data as separate variables in the data structure which in many cases is not necessary if we do it on the fly in the output process

I think this can be improved if we designate each of the tasks mentioned above to a separate object and make our final output process as a composition of them instead of a rigid derivation as can be seen in this figure:

outputprocess

As always, any opinion is welcomed!

@philbucher
Copy link
Member Author

closing after #8343

more steps can be added in followup PRs/Issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests