Morse Smale Quadrangulation point data arrays sometimes contains uninitialized values

[HollowSun0]

Describe the bug Sometimes, after a computation of ttkMorseSmaleQuadrangulation, the point data arrays "QuadCellId", "QuadVertType" and "ttkVertexScalarField" contains random values attributable to uninitialized values. See the following video:

https://www.dropbox.com/s/pyx2xoubgi5xbhj/SpecifX%203.0.0.DEV%202019-08-08%2016-47-40.mp4?dl=0

The beginning of the video starts right after the loading of a state file.

To Reproduce I have just followed your tutorial https://www.youtube.com/watch?v=eNW8l5PlgpU and got stuck at MorseSmaleQuadrangulation. As soon as I print all the values of std::vector outPointsIds(https://github.com/topology-tool-kit/ttk/blob/master/core/vtk/ttkMorseSmaleQuadrangulation/ttkMorseSmaleQuadrangulation.cpp, line 138) and vtkIntArray* Identifiers, the problem goes away.

Hypothesis: Maybe the slowdown caused by console printing fixes some form of race condition.

Expected behavior Sometimes, the values are good, but in other cases, it will cause the Qudrangulation Subdivision to crash. The problem seems to happen more often at the loading of the state file.

System (please complete the following information):

• Windows 10
• Visual Studio 2017
• VTK 8.2.0
• TTK 0.9.8

[julien-tierny]

hi there,

thanks for your message.

this one is for @pierre-guillou, who is the main author of this module. looks like some basic data race indeed.

cheers, -- Dr Julien Tierny CNRS Researcher Sorbonne Universite http://lip6.fr/Julien.Tierny

On Thursday, August 8, 2019 11:12:50 PM CEST Patrick Laurin wrote:

Describe the bug Sometimes, after a computation of ttkMorseSmaleQuadrangulation, the point data arrays "QuadCellId", "QuadVertType" and "ttkVertexScalarField" contains random values attributable to uninitialized values. See the following video:

https://www.dropbox.com/s/pyx2xoubgi5xbhj/SpecifX%203.0.0.DEV%202019-08-08%2016-47-40.mp4?dl=0

To Reproduce I have just followed your tutorial https://www.youtube.com/watch?v=eNW8l5PlgpU and got stuck at MorseSmaleQuadrangulation. As soon as I print all the values of std::vector outPointsIds(https://github.com/topology-tool-kit/ttk/blob/master/core/vtk/ttkMorseSmaleQuadrangulation/ttkMorseSmaleQuadrangulation.cpp, line 138) and vtkIntArray* Identifiers, the problem goes away.

Hypothesis: Maybe the slowdown caused by console printing fixes some form of race condition.

Expected behavior Sometimes, the values are good, but in other cases, it will cause the Qudrangulation Subdivision to crash.

System (please complete the following information):

• Windows 10
• Visual Studio 2017
• VTK 8.2.0
• TTK 0.9.8

[pierre-guillou]

Hi,

since I cannot seem to reproduce this bug on my Linux machine, it seems to me that this is a Windows/MSVC thing.

Since I will not have access to my workstation with my Windows virtual machine until next Wednesday (still on holidays…), I am not able to track down the problem for now.

Sorry for the inconvenience, Pierre

[pierre-guillou]

Hi @HollowSun0,

I am sorry to tell you that, so far, I have not been able to reproduce your bug, either on Windows or on Linux.

Since our OpenMP-based parallel code cannot be build on Windows with Visual Studio, I don't think this could be a data race issue.

I initially though that the auto & constructs used in https://github.com/topology-tool-kit/ttk/blob/master/core/vtk/ttkMorseSmaleQuadrangulation/ttkMorseSmaleQuadrangulation.cpp#L138 could be somewhat faulty and result in a local vector copy instead of a reference, leading to partially uninitialized memory after freeing the copied local vector at the end of the doIt method. But after checking with a small example, and re-building the whole TTK library with Visual Studio, it worked as intended.

In order to dive deeper into this problem, I need more information. Could you provide us with the output of the CMake configuration ? It will list all the TTK build options and will allow me to better replicate your configuration.

Additionally, could you sent us your faulty ParaView state file, assuming it differs from the one in https://github.com/topology-tool-kit/ttk-data/blob/master/states/morseSmaleQuadrangulation.pvsm?

Thank you for your help, and sorry for the inconvenience,

Pierre

[HollowSun0]

Sorry for the late response... I was quite busy for the last 3 weeks. Now...

Indeed, it did look like the problem you are mentioning was it but if it's not the case... I'll give you more info about my test case.

We are using wrappers on every TTK filter, but only this one seems to show problems. Here's the state file I've used to generate the problem. Since some property names of ttk plugins differ from my wrappers, I did my best to edit the .pvsm file by hand and match the property names to the source TTK plugins. The MorseSmaleQuadrangulation wrapper is also contained in the zip file, that should automatically work by loading the plugin in your ParaView.

MorseSmalequadrangulation.zip

Here are the CMake options used for compiling TTK:

ttk options ----------------------------------------------------------------- TTK_ENABLE_CPU_OPTIMIZATION: ON TTK_ENABLE_EIGEN: ON TTK_ENABLE_GRAPHVIZ: OFF TTK_ENABLE_KAMIKAZE: OFF TTK_ENABLE_MPI: OFF TTK_ENABLE_OPENMP: OFF TTK_ENABLE_SCIKIT_LEARN: OFF TTK_ENABLE_SPECTRA: ON TTK_ENABLE_SQLITE3: OFF TTK_ENABLE_ZFP: OFF TTK_ENABLE_ZLIB: OFF TTK_ENABLE_64BIT_IDS: OFF ttk build ------------------------------------------------------------------- CMAKE_BUILD_TYPE: TTK_BUILD_DOCUMENTATION: OFF TTK_BUILD_PARAVIEW_PLUGINS: OFF TTK_BUILD_STANDALONE_APPS: OFF TTK_BUILD_VTK_WRAPPERS: ON VTK_DIR: VTK_DIR-NOTFOUND ttk install ----------------------------------------------------------------- CMAKE_INSTALL_PREFIX: C:/Program Files/ttk TTK_INSTALL_PLUGIN_DIR:

[pierre-guillou]

Hi @HollowSun0,

Thanks for helping me tracking down this issue by sharing your code.

I managed to insert your wrapper code into TTK, build it and run it on my Linux machine. However, I got a segfault testing your PVSM state file at the end of the MorseSmaleQuadrangulationWrapper code that I think may be related to the current issue. Here's my take on it:

In your MorseQuadrangulationWrapper.cxx, you define a ttkMorseSmaleQuadrangulationFilter variable as smart pointer to our filter, to set some parameters, launch execution and copy the output. This smart pointer does not live after the end of the MorseSmaleQuadrangulationWrapper::RequestData function. Going out of scope, it may trigger the deletion of our ttkMorseSmaleQuadrangulation filter object, which holds the memory pointed by its output, leading to use-after-free run-time errors.

Indeed, we avoid a copy of our output data by pointing the output arrays of our ttkMorseSmaleQuadrangulation filter to standard vectors that are members of this class (or rather members of a member), through each call of the setVoidArray VTK method.

I used the setVoidArray method extensively in the MorseSmaleQuadrangulation, the QuadrangulationSubdivision and the BarycentricSubdivision filters to avoid unecessary copies. Apart from these filters (which were all written by me), the ttkDimensionReduction filter is the only one using this kind of memory management. Have you got some issues when using it through your wrapper?

A simple fix in your wrapper would be, I think, to promote this ttkMorseSmaleQuadrangulationFilter local variable as a class member. I tested it and I got no more segfaults. Could you try the same and report if it corrected this issue? It should either confirm or disprove my theory.

Pierre

[HollowSun0]

Hi Pierre,

I can confirm that your fix works for me too, which confirms your theory.

Knowing that information, I tried DeepCopying the output and realized that it also works. Now, I'm not too sure which method is best to use. Perhaps, I will follow the DeepCopy route.

By greping "SetVoidArray", I'm able to know which filters are avoiding unnecessary copies, and it will help me target the right filters and minimize the number of DeepCopies I need to do.

Thanks Pierre, I shall continue with this approach!

Have a good day,

Patrick Laurin