View Issue Details

IDProjectCategoryView StatusLast Update
0002213OpenFOAMPatchpublic2016-12-03 20:00
ReporterwyldckatAssigned Tohenry 
PrioritynormalSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
Product Version 
Fixed in Version 
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.

Relationships

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

Activities

wyldckat

2016-08-24 01:45

updater  

vtkTopo.C (12,063 bytes)

henry

2016-08-24 09:42

manager   ~0006734

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.

wyldckat

2016-08-24 11:08

updater   ~0006735

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?

henry

2016-08-24 11:15

manager   ~0006736

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.

wyldckat

2016-08-24 14:04

updater   ~0006738

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.

henry

2016-08-24 14:11

manager   ~0006739

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

wyldckat

2016-08-24 14:28

updater   ~0006740

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

henry

2016-08-24 14:35

manager   ~0006741

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.

wyldckat

2016-08-24 15:28

updater   ~0006742

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.

wyldckat

2016-12-03 20:00

updater   ~0007389

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

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