Support for Paraview/VTK XML (.vtu) and Multiblock (.vtm) visualization files #845

talbring · 2020-01-06T15:40:01Z

Proposed Changes

I added new output file formats. First of all support for the VTK XML file format (see this pdf) (file ending *.vtu). This adds support for possible compression, portable binary encoding, random access, big endian and little endian byte order, multiple file representation of piece data. I made this the new default format if PARAVIEW has been added to OUTPUT_FILES. The old *.vtk format can still be used with PARAVIEW_LEGACY.

However, the main reason for this new format is that it is required for new Multiblock file (*.vtm) output. This file is a simple XML file which defines a data hierarchy in order to represent individual parts of the domain (which are stored as *.vtu files). With this it is possible to directly select and visualize the solution in individual zones and/or on boundaries. This new format can be enabled with PARAVIEW_MULTIBLOCK as value for OUTPUT_FILES. At the moment all boundaries are written, I might add an option to disable/enable some of them (maybe re-using the MARKER_PLOTTING option). In order not to cluster the working directory, the individual *.vtu files are placed in subfolders, since they are more or less opaque to the user.

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

I am submitting my contribution to the develop branch.
My contribution generates no new compiler warnings (try with the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags).
My contribution is commented and consistent with SU2 style.
I have added a test case that demonstrates my contribution, if necessary.
I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

…the casename

talbring · 2020-01-07T18:03:19Z

The vtm file and the folder where the data is stored are now called like the config file (called "case name"). Futhermore the files inside of the folder are organized like the data. That means after a successful run, the working directory looks like that (omitting history files):

├── cht_2d_3cylinders
│   ├── zone_0
│   │   ├── cylinder_outer1.vtu
│   │   ├── cylinder_outer2.vtu
│   │   ├── cylinder_outer3.vtu
│   │   ├── farfield.vtu
│   │   └── Internal.vtu
│   ├── zone_1
│   │   ├── core1.vtu
│   │   ├── cylinder_inner1.vtu
│   │   └── Internal.vtu
│   ├── zone_2
│   │   ├── core2.vtu
│   │   ├── cylinder_inner2.vtu
│   │   └── Internal.vtu
│   └── zone_3
│       ├── core3.vtu
│       ├── cylinder_inner3.vtu
│       └── Internal.vtu
├── cht_2d_3cylinders.cfg
├── cht_2d_3cylinders.vtm
├── solid_cylinder1.cfg
├── solid_cylinder2.cfg
└── solid_cylinder3.cfg

And the structure in Paraview:

pcarruscag

Very useful feature! My usual nitpicking below :)

pcarruscag · 2020-01-08T20:55:04Z

Common/src/config_structure.cpp

+  /*--- Set the case name to the base config file name without extension ---*/
+
+  caseName = string(case_filename);
+  unsigned short lastindex = caseName.find_last_of(".");
+  caseName = caseName.substr(0, lastindex);
+


It would be nice to have this kind of common operation in a toolbox (whatever the stl does not offer).

pcarruscag · 2020-01-08T21:02:55Z

Common/src/config_structure.cpp

+  for (unsigned short iFiles = 0; iFiles < driver_config->GetnVolumeOutputFiles(); iFiles++){
+    if (driver_config->GetVolumeOutputFiles()[iFiles] == PARAVIEW_MULTIBLOCK){
+      multiblockDriver = true;
+    }
+  }


You can do std::any_of if you want to be more c++11

pcarruscag · 2020-01-08T21:04:32Z

SU2_CFD/include/output/filewriter/CFEMDataSorter.hpp

   */
-  CFEMDataSorter(CConfig *config, CGeometry *geometry, unsigned short nFields);
+  CFEMDataSorter(CConfig *config, CGeometry *geometry, vector<string> fieldNames);


Please pass fieldNames by const&

pcarruscag · 2020-01-08T21:08:27Z

SU2_CFD/src/output/COutput.cpp

+      
      if (fileName.empty())
        fileName = config->GetFilename(volumeFilename, "", curTimeIter);
-
+      
      /*--- Load and sort the output data and connectivity. ---*/
-
+      
      volumeDataSorter->SortConnectivity(config, geometry, true);
-
+      


Remember to strip all blank spaces :)

I wonder why CodeFactor did not complain

Reminder to remove whitespaces

pcarruscag · 2020-01-08T21:13:37Z

SU2_CFD/include/output/filewriter/CParallelDataSorter.hpp

+  vector<string> GetFieldNames(){
+    return fieldNames;
+  }


This can probably be returned by const&, it will still allow a local copy to be created if necessary.

pcarruscag · 2020-01-08T21:17:35Z

SU2_CFD/src/output/filewriter/CParaviewFileWriter.cpp

-    string fieldname = fieldnames[iField];
+    string fieldname = fieldNames[iField];

    fieldname.erase(remove(fieldname.begin(), fieldname.end(), '"'), fieldname.end());



Could be my jet lagged eyes but it looks like this local manipulation of the name is not used?

Can you confirm or deny my observation?

Its used a couple of lines below (line 287).

pcarruscag · 2020-01-08T21:20:48Z

SU2_CFD/src/output/filewriter/CParaviewVTMFileWriter.cpp

+CParaviewVTMFileWriter::CParaviewVTMFileWriter(string fileName, string folderName, su2double time, 
+                                               unsigned short iZone, unsigned short nZone)
+  : CFileWriter(std::move(fileName), fileExt),
+    folderName(std::move(folderName)), iZone(iZone), nZone(nZone), curTime(time){


I would differentiate the argument / member names here (IDK if compilers complain or not, but just in case).

pcarruscag · 2020-01-08T21:22:04Z

SU2_CFD/src/output/filewriter/CParaviewVTMFileWriter.cpp

+  if (rank == MASTER_NODE)
+#if defined(_WIN32) || defined(_WIN64) || defined (__WINDOWS__)
+    _mkdir(this->folderName.c_str());
+    _mkdir((this->folderName + "/zone_" + to_string(iZone)).c_str());
+#else 
+    mkdir(this->folderName.c_str(), 0777); 
+    mkdir((this->folderName + "/zone_" + to_string(iZone)).c_str(), 0777);
+#endif


Should we have a "SU2_mkdir" in a toolbox to hide the windows/linux "hacks"?

pcarruscag

Few more comments (I did not get to the bottom earlier).

pcarruscag · 2020-01-09T02:37:03Z

SU2_CFD/src/output/filewriter/CParallelFileWriter.cpp

+CFileWriter::CFileWriter(string fileName, string fileExt):
+  file_ext(fileExt),
+  fileName(std::move(fileName)){
+
+  rank = SU2_MPI::GetRank();
+  size = SU2_MPI::GetSize();
+
+  this->fileName += fileExt;
+
+  file_size = 0.0;
+  Bandwidth = 0.0;
+
+}


Same comment on differentiating names as before, + try to be consistent with the naming convention (underscores vs camel case).

pcarruscag · 2020-01-09T02:41:56Z

SU2_CFD/src/output/filewriter/CParaviewBinaryFileWriter.cpp

+
+  const vector<string> fieldNames = dataSorter->GetFieldNames();


If GetFieldNames returned by const& (see one of the previous comments) this copy could be avoided.
(I know it is not critical, it is just a matter of following a convention that works well when such aspects are important).

pcarruscag · 2020-01-09T02:43:22Z

SU2_CFD/src/output/filewriter/CParaviewBinaryFileWriter.cpp


-    string fieldname = fieldnames[iField];
+    string fieldname = fieldNames[iField];


Same comment as before, this variable not being used.

pcarruscag · 2020-01-09T02:45:39Z

SU2_CFD/src/output/filewriter/CParaviewVTMFileWriter.cpp

+      multiBlockFile.open (fileName.c_str());
+    else 
+      multiBlockFile.open(fileName.c_str(), ios::app);


The .c_str() is not needed in c++11, there is an overload for string.

pcarruscag · 2020-01-09T02:55:35Z

SU2_CFD/src/output/filewriter/CParaviewXMLFileWriter.cpp

+  for (iElem = 0; iElem < nParallel_Line; iElem++) {
+    type_buf[jElem] = LINE; jElem++;
+  }
+  for (iElem = 0; iElem < nParallel_Tria; iElem++) {
+    type_buf[jElem] = TRIANGLE; jElem++;
+  }


You could use std::fill to make this sections of code more expressive (one line instead of three).

pcarruscag · 2020-01-09T02:58:54Z

SU2_CFD/src/output/filewriter/CParaviewXMLFileWriter.cpp

+
+      /*--- Prepare the 1D data buffer on this rank. ---*/
+
+      float *vec_buf = new float[myPoint*NCOORDS];


I would allocate this buffer once outside the loop over variables and re-use it also for the scalar case, since the peak memory usage should not change.

pcarruscag · 2020-01-09T03:04:31Z

SU2_CFD/src/output/filewriter/CParaviewXMLFileWriter.cpp

+  std::string typeStr;
+  unsigned long typeSize;
+  switch (type) {
+    case VTKDatatype::FLOAT32:
+      typeStr = "\"Float32\"";
+      typeSize = sizeof(float);
+      break;
+    case VTKDatatype::INT32:
+      typeStr = "\"Int32\"";
+      typeSize = sizeof(int);
+      break;
+    case VTKDatatype::UINT8:
+      typeStr = "\"UInt8\"";
+      typeSize = sizeof(char);     
+      break;
+    default:
+      SU2_MPI::Error("Unknown Type", CURRENT_FUNCTION);
+      break;
+  }


This is done also in AddDataArray, maybe it could be in a function to make maintenance easier if more types are added?

pcarruscag · 2020-01-09T03:06:51Z

SU2_CFD/src/output/filewriter/CSurfaceFVMDataSorter.cpp

@@ -1119,7 +1133,31 @@ void CSurfaceFVMDataSorter::SortConnectivity(CConfig *config, CGeometry *geometr

 }

-void CSurfaceFVMDataSorter::SortSurfaceConnectivity(CConfig *config, CGeometry *geometry, unsigned short Elem_Type) {
+void CSurfaceFVMDataSorter::SortConnectivity(CConfig *config, CGeometry *geometry, vector<string> markerList) {


Another vector that should be passed by const& here and in SortSurfaceConnectivity.

talbring · 2020-01-24T13:00:08Z

Problems are solved. PR is ready from my side.

pcarruscag

Looks very very clean, I did notice the changes, thanks!
Few more comments below, I think only one might be important.

pcarruscag · 2020-01-24T13:50:30Z

SU2_CFD/include/output/filewriter/CFileWriter.hpp

+#ifdef HAVE_MPI
+  /*!
+   * \brief The displacement that every process has in the current file view
+   */
+  MPI_Offset disp;
+
+  /*!
+   * \brief The file handle for writing
+   */
+  MPI_File fhw;
+#else
+
+  /*!
+   * \brief The displacement that every process has in the current file view
+   */
+  unsigned long disp;
+
+  /*!
+   * \brief The file handle for writing
+   */
+  FILE* fhw;
+#endif


Can these be typedefs of the MPI wrapper?

pcarruscag · 2020-01-24T13:51:38Z

SU2_CFD/include/output/filewriter/CParallelDataSorter.hpp

+  bool connectivitySorted;            //!< Boolean to store information on whether the connectivity is sorted

  int *nPoint_Send;                    //!< Number of points this processor has to send to other processors
  int *nPoint_Recv;                    //!< Number of points this processor receives from other processors
+  int *nElem_Send;                     //!< Number of elements this processor has to send to other processors
+  int *nElem_Cum;                     //!< Cumulative number of elements
+  int *nElemConn_Send;                     //!< Number of element connectivity this processor has to send to other processors
+  int *nElemConn_Cum;                     //!< Cumulative number of element connectivity entries


Would be a shame to break the pretty alignment :)

pcarruscag · 2020-01-24T13:54:15Z

SU2_CFD/include/output/filewriter/CParaviewXMLFileWriter.hpp

+   * \param[out] typeStr - The string name of the type
+   * \param[out] typeSize - The size in bytes of the type
+   */
+  inline void GetTypeInfo(const VTKDatatype type, string &typeStr, unsigned long &typeSize){


nitpick this method can be const

pcarruscag · 2020-01-24T13:58:44Z

SU2_CFD/src/output/filewriter/CParallelDataSorter.cpp

+  if (nElem_Send != NULL) delete []  nElem_Send;
+  if (nElem_Cum != NULL) delete []  nElem_Cum;
+  if (nElemConn_Send != NULL) delete []  nElemConn_Send;
+  if (nElemConn_Cum != NULL) delete []  nElemConn_Cum;
+


This thing we do blabla != NULL is actually not needed. (I only looked for the definitive answer recently)

pcarruscag · 2020-01-24T14:00:36Z

SU2_CFD/src/output/filewriter/CParallelDataSorter.cpp

+  nGlobalElem = std::accumulate(nGlobalPerElem.begin(), nGlobalPerElem.end(), 0); 
+  nLocalElem  = std::accumulate(nLocalPerElem.begin(), nLocalPerElem.end(), 0);


Ah yes... me likes.

pcarruscag · 2020-01-24T14:01:48Z

SU2_CFD/src/output/filewriter/CParallelFileWriter.cpp

+#else
+
+  startTime = su2double(clock())/su2double(CLOCKS_PER_SEC);
+
+  unsigned long bytesWritten;


I have my eye on moving this timing stuff to the wrapper at some point.

pcarruscag · 2020-01-24T14:03:39Z

SU2_CFD/src/output/filewriter/CParaviewFileWriter.cpp

-    string fieldname = fieldnames[iField];
+    string fieldname = fieldNames[iField];

    fieldname.erase(remove(fieldname.begin(), fieldname.end(), '"'), fieldname.end());



Can you confirm or deny my observation?

pcarruscag · 2020-01-24T14:05:03Z

SU2_CFD/src/output/filewriter/CParaviewVTMFileWriter.cpp

+  if (rank == MASTER_NODE)
+#if defined(_WIN32) || defined(_WIN64) || defined (__WINDOWS__)
+    _mkdir(this->folderName.c_str());
+    _mkdir((this->folderName + "/zone_" + to_string(iZone)).c_str());
+#else
+    mkdir(this->folderName.c_str(), 0777);
+    mkdir((this->folderName + "/zone_" + to_string(valiZone)).c_str(), 0777);
+#endif


I did not get enough sleep but I don't think you have enough "{}"

oh yes you are right :D fortunately if the folders already exists, _mkdir() just does nothing. But I am gonna change this.

pcarruscag · 2020-01-24T14:09:45Z

SU2_DOT/src/meson.build

+                                         'output/COutput.cpp',
+                                         'output/output_structure_legacy.cpp',
+                                				 'output/tools/CWindowingTools.cpp',
+                                         'output/output_structure_legacy.cpp',
+                                				 'output/CBaselineOutput.cpp',
+                                				 'output/filewriter/CParallelDataSorter.cpp',
+                                				 'output/filewriter/CParallelFileWriter.cpp',
+                                				 'output/filewriter/CFEMDataSorter.cpp',
+                                				 'output/filewriter/CSurfaceFEMDataSorter.cpp',
+                                				 'output/filewriter/CFVMDataSorter.cpp',


Please align these.

pcarruscag · 2020-01-24T14:11:23Z

SU2_SOL/src/meson.build

                                        'output/filewriter/CSU2MeshFileWriter.cpp',
+                                        'output/filewriter/CParaviewXMLFileWriter.cpp',
+                                        'output/filewriter/CParaviewVTMFileWriter.cpp',
                                        'variables/CBaselineVariable.cpp',


I might have missed it, but can you add these to the old build system too?

I think this is not addressed yet.

TobiKattmann

Only formal aspects . I would like to have them addressed though

SU2_CFD/include/output/filewriter/CParallelDataSorter.hpp

SU2_CFD/include/output/filewriter/CSurfaceFEMDataSorter.hpp

SU2_CFD/include/output/filewriter/CSurfaceFVMDataSorter.hpp

SU2_CFD/src/output/COutput.cpp

TobiKattmann · 2020-01-27T14:40:03Z

SU2_CFD/src/output/filewriter/CFEMDataSorter.cpp


-#ifdef HAVE_MPI
-  SU2_MPI::Allreduce(&nLocalPoint_Sort, &nGlobalPoint_Sort, 1,
+  SU2_MPI::Allreduce(&nLocalPointBeforeSort, &nGlobalPointBeforeSort, 1,
                     MPI_UNSIGNED_LONG, MPI_SUM, MPI_COMM_WORLD);


👍 for removing the preprocessor stuff

SU2_DOT/src/meson.build

TobiKattmann

I am really looking forward to use this (especially for multizone cases) because I am certain this will make visualization (and post-processing in general) in Paraview much more comfortable..

All cases I tested are now running without issues and I do not have any further review comments, so LGTM 💐

…ure_multiblock

pcarruscag · 2020-01-28T09:01:24Z

SU2_CFD/obj/Makefile.am

+  ../src/output/filewriter/CParaviewXMLFileWriter.cpp \
+  ../src/output/filewriter/CParaviewVTMFileWriter.cpp \


Thanks LGTM

talbring added 23 commits January 5, 2020 23:34

Adding new config enums and files

4095025

Error checking for multizone and multiblock output

24dfc64

New filewriter constructor

fd0f845

Added new routine to sort specified markers

a45d7a7

Adding VTM filewriter header

3940056

Adding VTK XML filewriter header

6fc9e04

Adding files to meson

dccbb71

adding new file cases to output class

598f262

Adding vtm filewriter implementation

8bf4ea1

Adding vtk xml writer implementation

22b9f0a

Added missing meson entries

ad9969b

Added time value and data set writing to vtm output

f288783

Updated vtk xml writer for serial compilation

b7e1861

Updated WriteToFile routine

faad7dd

Small fix for AD compilation

e6c443c

adding case name field to config

c12c3f9

Removed redundant information from filewriter/sorting routines

b1bfc3e

Adapted allocation of filewriters/sorting routines

348fca0

Added more comments to multiblock section and changed filename to be …

68abcfc

…the casename

Changed folder name to case name and moved zone data to folder

2fa3dfc

Added multizone name to vtm file

166ce1a

Some fixes

08f5f3b

Adding comments

778a6f0

talbring changed the title ~~WIP Support for Paraview/VTK XML (*.vtu) and Multiblock (*.vtm) visualization files~~ Support for Paraview/VTK XML (*.vtu) and Multiblock (*.vtm) visualization files Jan 7, 2020

pr-triage bot added the PR: unreviewed label Jan 7, 2020

talbring added 2 commits January 7, 2020 13:50

Fix for windows compilation

37770b4

Added const and missing windows includes

e89a298

pcarruscag reviewed Jan 8, 2020

View reviewed changes

pcarruscag reviewed Jan 9, 2020

View reviewed changes

talbring added 2 commits January 24, 2020 11:36

Adding bandwidth output

f03acf7

Removing some code duplication

7bdbc1d

talbring changed the title ~~WIP Support for Paraview/VTK XML (*.vtu) and Multiblock (*.vtm) visualization files~~ Support for Paraview/VTK XML (*.vtu) and Multiblock (*.vtm) visualization files Jan 24, 2020

pr-triage bot added the PR: unreviewed label Jan 24, 2020

Merge branch 'develop' into feature_multiblock

cdef521

pcarruscag reviewed Jan 24, 2020

View reviewed changes

pcarruscag and others added 4 commits January 24, 2020 14:17

Merge branch 'develop' into feature_multiblock

53847b0

Small fixes

a0568f7

Fixed alignment

b78b443

Fixed memory leak

b636f09

TobiKattmann reviewed Jan 27, 2020

View reviewed changes

talbring added 3 commits January 27, 2020 16:31

Fixed some comments and renamed some variables

253fc16

Fixed alignment

a1dbcaa

Merge branch 'develop' into feature_multiblock

1871754

TobiKattmann approved these changes Jan 28, 2020

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Jan 28, 2020

talbring added 2 commits January 28, 2020 09:30

Updating automake files

afc203d

Merge branch 'feature_multiblock' of github.com:su2code/SU2 into feat…

318acd1

…ure_multiblock

pr-triage bot added PR: unreviewed and removed PR: reviewed-approved labels Jan 28, 2020

pcarruscag approved these changes Jan 28, 2020

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Jan 28, 2020

pcarruscag merged commit eb28158 into develop Jan 28, 2020

pcarruscag deleted the feature_multiblock branch January 28, 2020 22:39

pr-triage bot added PR: merged and removed PR: reviewed-approved labels Jan 28, 2020

talbring mentioned this pull request May 20, 2020

Different zones for different MARKER_PLOTTING #995

Closed


		const vector<string> fieldNames = dataSorter->GetFieldNames();


		string fieldname = fieldnames[iField];
		string fieldname = fieldNames[iField];


		/--- Prepare the 1D data buffer on this rank. ---/

		float vec_buf = new float[myPointNCOORDS];

		nGlobalElem = std::accumulate(nGlobalPerElem.begin(), nGlobalPerElem.end(), 0);
		nLocalElem = std::accumulate(nLocalPerElem.begin(), nLocalPerElem.end(), 0);

		../src/output/filewriter/CParaviewXMLFileWriter.cpp \
		../src/output/filewriter/CParaviewVTMFileWriter.cpp \

Support for Paraview/VTK XML (*.vtu) and Multiblock (*.vtm) visualization files #845

Support for Paraview/VTK XML (*.vtu) and Multiblock (*.vtm) visualization files #845

Conversation

talbring commented Jan 6, 2020 • edited Loading

Proposed Changes

PR Checklist

talbring commented Jan 7, 2020

pcarruscag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcarruscag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

talbring commented Jan 24, 2020

pcarruscag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TobiKattmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TobiKattmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Support for Paraview/VTK XML (.vtu) and Multiblock (.vtm) visualization files #845

Support for Paraview/VTK XML (.vtu) and Multiblock (.vtm) visualization files #845

talbring commented Jan 6, 2020 •

edited

Loading