Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description and Context
This addresses #134 by providing the option to export the simulation output to a .vti file. The CMake variable
MIRCO_ENABLE_VISUALIZATIONEXPORTneeds to be turned ON to enable simulation export. Then, one can set the following two parameters in the .yaml file to export the visualization:I made enabling simulation an extra step in CMake so that VTK is not required if one does not need visualizations.
I went back and forth between either using some sort of struct which I pass to
Evaluate(), and then doing the visualization in main(), or just doing the visualization export inEvaluate(). I suppose the latter has the benefit that visualizations can also be exported when MIRCO is used as a library. But I'm not entirely sure about this structure, as it mixes the visualization export in with the actual computation..Also, I made a change in the
CMakeLists.txtsuch that the public headers are now explicitly listed. I think this is standard practice, as not all headers need to be accessible from outside MIRCO.P.S. It currently shows the shape factor computation commits here because I rebased onto that branch. I think this will be resolved when that PR (#138) is merged.
Related Issues and Pull Requests
Some pictures of the visualization in Paraview:
Interested Parties / Possible Reviewers
@mayrmt
@sbrandstaeter
@RShaw026