View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002213 | OpenFOAM | Patch | public | 2016-08-24 01:45 | 2016-12-03 20:00 |
| Reporter | wyldckat | Assigned To | henry | ||
| Priority | normal | Severity | minor | Reproducibility | sometimes |
| Status | resolved | Resolution | fixed | ||
| Product Version | |||||
| Fixed in Version | |||||
| Summary | 0002213: "vtkTopo.C" in foamToVTK library is missing the same fix as provided in #1633 | ||||
| Description | While 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 Information | Note: 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"? | ||||
| Tags | No tags attached. | ||||
|
|
vtkTopo.C (12,063 bytes) |
|
|
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. |
|
|
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? |
|
|
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. |
|
|
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. |
|
|
Should the remaining VTK exports be moved into the foamToVTK library and that library linked in when VTK output is needed? |
|
|
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 |
|
|
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. |
|
|
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. |
|
|
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 |
| 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 |
