Rewrite libs/vtkh/vtkm_filters to use new vtk-m wrappers
Some files in vtkm_filters point to worklets provided by vtk-m, this is not ideal since it bloats the build size for redundant code. VTK-m has gone through the process of providing wrappers for their filters that we should use instead.
Issue isn't vtkm_filters. These are, for the most part, friendly wrappers that call vtkm functions that utilize vtkm worklets. With the exception of Threshold, which needs to have its worklet migrated to vtkm.
The majority of the problem children are in vtkh/filters. These directly utilize vtkm worklets and thus require the device compiler flags (this was a problem with amdgpu+kokkos and amr-wind and this will presumably be a fix?). Need to migrate these to vtkm, then write wrappers for the migrated worklets in vtkh/vtkm_filters, and then call wrapped functions in vtkh/filters. This is to follow the current format. Problem children: vtkh/vtkm_filters/threshold vtkh/filters/HistSampling vtkh/filters/Statistics vtkh/filters/CompositeVector vtkh/filters/Log vtkh/filters/GhostStripper vtkh/filters/Slice vtkh/filters/VectorMagnitude vtkh/filters/Clip vtkh/filters/Histogram
Note: vtkh is located in ascent/src/libs/
- The
worklet::CellDeepCopyinvtkh/vtkm_filters/thresholdcould be replaced by theCleanGridfilter. - The
vtkh/filter/Clipjust added theMultiPlaneImplicitFunctiontoClipWithImplicitFunctionin a very ugly way. TheMultplaneImplicitFunctioncould be added to vtk-m proper. -
vtkh/filter/Histogramshould usevtkm/filter/density_estimate/Histogramas stated in the comment. This could be an exercise for Ascent developers. - Same as
vtkh/filter/VectorMagnitude. -
vtkh/filter/Loglooks like an exercise/example. Do we really need it? -
vtkh/filter/CompositeVectorcould/should be replace by vtk-m'sArrayHandleComposite. -
vtkh/filter/Statisticsshould use the newvtkm/worklet/DescriptiveStatisticsinstead of callingAlgorithm::Reduceetc. -
vtkh/filter/HistSamplingshould also be rewritten to use new random number generator and other new facilities in vtk-m.
I was writing my response at the same time as @ollielo. Most of these are probably already implemented in VTK-m:
- threshold: Are you referring to the use of
vtkm/worklet/CellDeepCopy.hinvtkmThreshold.cpp? Using this should no longer be necessary. Just usevtkm::filter::entity_extraction::Thresholdand you should be good. - GhostStripper:
vtkm::filter::entity_extraction::GhostCellRemove - Slice:
vtkm::filter::contour::Slice - VectorMagnitude:
vtkm::filter::vector_analysis::VectorMagnitude - Clip:
vtkm::filter::contour::ClipWithImplicitFunction - Histogram:
vtkm::filter::density_estimate::Histogram
This just leaves:
- HistSampling: I'm not sure what this does. Does it do a histogram of a random sample of the data? Is that really that much faster than making a single pass through the array?
- Statistics: There is no filter that does statistics in VTK-m, but there is already some worklet code for it. I expect it to be pretty trivial to wrap that in a filter.
- CompositeVector: I see why you would want this in a filter instead of using
ArrayHandleComposite. A basic implementation should be pretty easy, but there are lots of special cases that would make this interesting. - Log: Trivial to implement (assuming you still want it).
@kmorel
vtkm::filter::entity_extraction::Threshold doesn't do Compaction, it still returns a CellSetPermutation. If the goal is to remove/strip CellSetPermutation, a CleanGrid pass is still needed.
HistSampling does random sampling according to some kind of probability density function (PDF). The point is to sample less from high probability region and more on the low-probability region (to maximize the entropy of the sample).
Huh. I thought we changed that because a DataSet with. CellSetPermutation is all but useless. There should definitely be an option in Threshold (defaulted to true) that internally runs the result through CleanGrid.
Adding CleanGrid to Threshold could also be an exercise for the new comer. There are plenty examples out there (e.g. ThresholdPoints and ExtracePoints).
Correction, the Threshold does call CellDeepCopy deeply in its worklet:
https://gitlab.kitware.com/vtk/vtk-m/-/blob/master/vtkm/filter/entity_extraction/worklet/Threshold.h#L173
Thanks. So it looks like Threshold is complete. Correct?
Yes. The only thing you need to do is to remove the StripPermutation.
After a short discussion with @dpugmire, we try to start with the Log filter. Firstly, we try to add a Log filter in the vtkm (assuming it is under the field_conversion dir, maybe we could set the base number as a parameter for the filter), then in the current vtkh log filter, we call that vtkm log filter directly. Does it makes sense for you?
Btw, should we implement CellLog filter and PointLog filter separately, or just put them together following the current vtkh example (such as this one https://github.com/Alpine-DAV/ascent/blob/develop/src/libs/vtkh/filters/Log.cpp#L459). Thanks!
@wangzhezhe
It should be field_transform which transform fields from/to the same entity, instead of field_conversion which does something cell to point or point to cell conversion.
You really don't need separate CellLog and PointLog. The vtk-m infrastructure code can make one filter working for both cell and point field. I personally think it should also work for other field association (WHOLE_CELLSET) but it is up to you if you want it that way.
BTW, the way that there are three filters doing essentially the same thing is not ideal. IMHO, the best way to correct it is something similar to the vtkm::ImplicitFunctionGeneral which uses Variant to select and hold different ImplicitFunctions.
Thanks a lot for the information, it helps a lot ! @ollielo
@dpugmire -- related to your question during telecon.
Here is the running list of VTK-h to VTK-m conversion details.
It's not 100% complete, but it's a good place to pick up the discussion.
After several discussions with @nicolemarsaglia, we adapted the associated code from vtk-h and added several filters into vtk-m. Thanks a lot for the help from @kmorel, @dpugmire and @ollielo .
Log values https://gitlab.kitware.com/vtk/vtk-m/-/merge_requests/2852 Statistics filter https://gitlab.kitware.com/vtk/vtk-m/-/merge_requests/2957 Composite filter https://gitlab.kitware.com/vtk/vtk-m/-/merge_requests/2962 Distributed statistics filter https://gitlab.kitware.com/vtk/vtk-m/-/merge_requests/3017 Adaptive sampling https://gitlab.kitware.com/vtk/vtk-m/-/merge_requests/3083 MultiPlane https://gitlab.kitware.com/vtk/vtk-m/-/merge_requests/3124 SliceMultiple (3d slices) https://gitlab.kitware.com/vtk/vtk-m/-/merge_requests/3135
Thanks @wangzhezhe for all your effort getting these filters rehomed, and for making this list!
I'm going to keep this issue open until I've fully finished Ascent's implementation.
Thanks for the help! @nicolemarsaglia, just let me know if there are anything that I could help with!