2017-07-23 13:36 BST

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002213OpenFOAMPatchpublic2016-12-03 20:00
Reporterwyldckat 
Assigned Tohenry 
PrioritynormalSeverityminorReproducibilitysometimes
StatusresolvedResolutionfixed 
Product Version4.x 
Target VersionFixed in Version4.x 
Summary0002213: "vtkTopo.C" in foamToVTK library is missing the same fix as provided in #1633
DescriptionWhile diagnosing issue #2099, I stumbled upon a missing fix for "applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK/vtkTopo.C", which was applied to "vtkPV?FoamMeshVolume.C" in issue #1633 (commit e199a0fbf67), but which I completely forgot to test and provide the same fix for this "vtkTopo.C" file.

The odd thing is that I thought that I had seen this be already fixed sometime ago... but I might have mistaken it for something else... either that, or we have an additional class where this kind of FOAM to VTK format is being done...

Anyway, attached is the file "vtkTopo.C" for updating "applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK/vtkTopo.C" on both 4.x and dev, using the same fix as used in commit e199a0fbf67.
Additional InformationNote: This FOAM to VTK mesh conversion mechanism needs to be formalized into a single class, even if it's just a class of inline static methods for 1-to-1 cell conversions... or one from which the others derive...
Problem is: Where exactly should we place such a class? Maybe in "src/fileFormats/vtk"?
TagsNo tags attached.
Attached Files

-Relationships
related to 0002099resolvedhenry snap stage generates internal triangles 
related to 0002219new Proposition for deprecating the representation of tetWedge with a collapsed VTK_WEDGE when exporting to VTK 
+Relationships

-Notes

~0006734

henry (manager)

Thanks for fix, I am putting it in now.

I totally that this complex code dumlication is mental and needs to be sorted out. One option would be to keep all the generic functionality for foam to VTK in the foamToVTK library:

./applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK

and link this library into the ParaView reader.

~0006735

wyldckat (updater)

Then shouldn't the library code for "foamToVTK" be moved to "src", so that it's easier to be picked up by other applications/libraries?

~0006736

henry (manager)

I thought about that but it is not a general purpose library, it is currently only used for the foamToVTK application and potentially the ParaView reader. Another option would be to move the ParaView reader into the dataConversion/foamToVTK but that would separate it from the other ParaView readers.

~0006738

wyldckat (updater)

Consolidating the remaining VTK exports comes to mind, as a reason for moving the foamToVTK library to somewhere at "FOAM_SRC". For example, the sampling library and all other function objects that can export to VTK.

Nonetheless, for the time being, I'm not yet seeing a way to have a complete generalization for both on RAM (e.g. ParaView) and file export. Hence the simpler class of static methods, which does justify keeping the foamToVTK library source code where it is for now.

~0006739

henry (manager)

Should the remaining VTK exports be moved into the foamToVTK library and that library linked in when VTK output is needed?

~0006740

wyldckat (updater)

With some additional code revising (better safe than sorry), yes, I believe it would be best to consolidate the classes and code for exporting mesh/data to VTK into a single library, even though it may create a dependency on a larger library for all others that depend on that code. It would help focus any exporting efforts for VTK into a single library, as well as help enforce non-format-bound export mechanisms, like we currently use for function objects (forces library comes to mind, can't export to CSV or any other format).

The "src/fileFormats" is also a library that could take some further extending, so it would more easily cover formats that are already being handled up somewhere else up to a point.

Merging the two libraries probably doesn't make sense, but it does make sense to have "fileFormats" as an entry point for all export/import of external formats, even though this might not be achievable in the near future... but having mesh/data import/export to any format accessible from a single library, is something that could make it easier to have cooperative data transfer with other software (remote co-processing of some VTK files comes to mind; or for boundary condition coupling of large experimental databases).

Possible directory structure:

  - src
      - fileFormats
          - fileFormats
          - foamToVTK

~0006741

henry (manager)

Given that the 'foamToVTK' would be a sub-directory of 'fileFormats' it would be logical to rename it 'VTK' as it relates to the VTK format and the 'foamTo' is implied by its location.

~0006742

wyldckat (updater)

Just a few reminders:

  1- The "fileFormats" library already handles read and write operations, so it's not as straight forward to assume that the "foamTo" is implied.

  2- "src/fileFormats/vtk" has a reading class for VTK unstructured meshes, so it might make sense to move it into the consolidated library.

  3- Be careful to not use "libVTK.so" as the name for the final library binary.

~0007389

wyldckat (updater)

I've made a note to look into this later on, namely the consolidation of the VTK features into a single library, as discussed in the previous comments.

This report is hereby "resolved" as "fixed", as listed below:

Resolved in OpenFOAM-4.x by commit 85daa4f41a76d16ae8e6428d0ec81ccd583300b4

Resolved in OpenFOAM-dev by commit f50d72786360b6d37fb4a397d5acf7f42f353cd1
+Notes

-Issue History
Date Modified Username Field Change
2016-08-24 01:45 wyldckat New Issue
2016-08-24 01:45 wyldckat Status new => assigned
2016-08-24 01:45 wyldckat Assigned To => henry
2016-08-24 01:45 wyldckat File Added: vtkTopo.C
2016-08-24 01:46 wyldckat Relationship added related to 0002099
2016-08-24 09:42 henry Note Added: 0006734
2016-08-24 11:08 wyldckat Note Added: 0006735
2016-08-24 11:15 henry Note Added: 0006736
2016-08-24 14:04 wyldckat Note Added: 0006738
2016-08-24 14:11 henry Note Added: 0006739
2016-08-24 14:28 wyldckat Note Added: 0006740
2016-08-24 14:35 henry Note Added: 0006741
2016-08-24 15:28 wyldckat Note Added: 0006742
2016-08-26 00:59 wyldckat Relationship added related to 0002219
2016-12-03 20:00 wyldckat Status assigned => resolved
2016-12-03 20:00 wyldckat Resolution open => fixed
2016-12-03 20:00 wyldckat Fixed in Version => 4.x
2016-12-03 20:00 wyldckat Note Added: 0007389
+Issue History