diff --git a/docs/changelog/ghost-cell-remove-interface.md b/docs/changelog/ghost-cell-remove-interface.md new file mode 100644 index 000000000..68ccc20bb --- /dev/null +++ b/docs/changelog/ghost-cell-remove-interface.md @@ -0,0 +1,10 @@ +# Updated the interface and documentation of GhostCellRemove + +The `GhostCellRemove` filter had some methods inconsistent with the naming +convention elsewhere in VTK-m. The class itself was also in need of some +updated documentation. Both of these issues have been fixed. + +Additionally, there were some conditions that could lead to unexpected +behavior. For example, if the filter was asked to remove only ghost cells +and a cell was both a ghost cell and blank, it would not be removed. This +has been updated to be more consistent with expectations. diff --git a/vtkm/filter/entity_extraction/GhostCellRemove.cxx b/vtkm/filter/entity_extraction/GhostCellRemove.cxx index 1d6a8ffb7..465c5dacb 100644 --- a/vtkm/filter/entity_extraction/GhostCellRemove.cxx +++ b/vtkm/filter/entity_extraction/GhostCellRemove.cxx @@ -19,37 +19,33 @@ namespace { -class RemoveAllGhosts + +template +VTKM_EXEC inline bool ShouldRemove(T value, vtkm::UInt8 removeTypes) +{ + return ((value & removeTypes) != 0); +} + +class RemoveGhostPredicate { public: - VTKM_CONT - RemoveAllGhosts() = default; - - VTKM_EXEC bool operator()(const vtkm::UInt8& value) const { return (value == 0); } -}; - -class RemoveGhostByType -{ -public: - VTKM_CONT - RemoveGhostByType() - : RemoveType(0) + VTKM_CONT RemoveGhostPredicate() + : RemoveTypes(0xFF) { } - VTKM_CONT - explicit RemoveGhostByType(const vtkm::UInt8& val) - : RemoveType(static_cast(~val)) + VTKM_CONT explicit RemoveGhostPredicate(vtkm::UInt8 val) + : RemoveTypes(val) { } VTKM_EXEC bool operator()(const vtkm::UInt8& value) const { - return value == 0 || (value & RemoveType); + return !ShouldRemove(value, this->RemoveTypes); } private: - vtkm::UInt8 RemoveType; + vtkm::UInt8 RemoveTypes; }; template @@ -87,10 +83,9 @@ class RealMinMax : public vtkm::worklet::WorkletMapField { public: VTKM_CONT - RealMinMax(vtkm::Id3 cellDims, bool removeAllGhost, vtkm::UInt8 removeType) + RealMinMax(vtkm::Id3 cellDims, vtkm::UInt8 removeTypes) : CellDims(cellDims) - , RemoveAllGhost(removeAllGhost) - , RemoveType(removeType) + , RemoveTypes(removeTypes) { } @@ -121,8 +116,10 @@ public: VTKM_EXEC void operator()(const T& value, const vtkm::Id& index, AtomicType& atom) const { // we are finding the logical min max of valid cells - if ((RemoveAllGhost && value != 0) || (!RemoveAllGhost && (value != 0 && value | RemoveType))) + if (ShouldRemove(value, this->RemoveTypes)) + { return; + } vtkm::Id3 logical = getLogical(index, CellDims); @@ -137,8 +134,7 @@ public: private: vtkm::Id3 CellDims; - bool RemoveAllGhost; - vtkm::UInt8 RemoveType; + vtkm::UInt8 RemoveTypes; }; template @@ -166,13 +162,9 @@ class Validate : public vtkm::worklet::WorkletMapField { public: VTKM_CONT - Validate(const vtkm::Id3& cellDims, - bool removeAllGhost, - vtkm::UInt8 removeType, - const vtkm::RangeId3& range) + Validate(const vtkm::Id3& cellDims, vtkm::UInt8 removeTypes, const vtkm::RangeId3& range) : CellDims(cellDims) - , RemoveAll(removeAllGhost) - , RemoveVal(removeType) + , RemoveVals(removeTypes) , Range(range) { } @@ -181,33 +173,31 @@ public: typedef void ExecutionSignature(_1, InputIndex, _2); template - VTKM_EXEC void operator()(const T& value, const vtkm::Id& index, vtkm::UInt8& valid) const + VTKM_EXEC void operator()(const T& value, const vtkm::Id& index, vtkm::UInt8& invalid) const { - valid = 0; - if (RemoveAll && value == 0) - return; - else if (!RemoveAll && (value == 0 || (value & RemoveVal))) - return; - - if (checkRange(Range, getLogical(index, CellDims))) - valid = static_cast(1); + if (ShouldRemove(value, this->RemoveVals) && + checkRange(Range, getLogical(index, CellDims))) + { + invalid = static_cast(1); + } + else + { + invalid = 0; + } } private: vtkm::Id3 CellDims; - bool RemoveAll; - vtkm::UInt8 RemoveVal; + vtkm::UInt8 RemoveVals; vtkm::RangeId3 Range; }; template bool CanStrip(const vtkm::cont::ArrayHandle& ghostField, const vtkm::cont::Invoker& invoke, - bool removeAllGhost, - vtkm::UInt8 removeType, + vtkm::UInt8 removeTypes, vtkm::RangeId3& range, - const vtkm::Id3& cellDims, - vtkm::Id size) + const vtkm::Id3& cellDims) { vtkm::cont::ArrayHandle minmax; minmax.Allocate(6); @@ -218,18 +208,17 @@ bool CanStrip(const vtkm::cont::ArrayHandle& ghostField, minmax.WritePortal().Set(4, std::numeric_limits::min()); minmax.WritePortal().Set(5, std::numeric_limits::min()); - invoke(RealMinMax<3>(cellDims, removeAllGhost, removeType), ghostField, minmax); + invoke(RealMinMax<3>(cellDims, removeTypes), ghostField, minmax); auto portal = minmax.ReadPortal(); range = vtkm::RangeId3( portal.Get(0), portal.Get(3), portal.Get(1), portal.Get(4), portal.Get(2), portal.Get(5)); - vtkm::cont::ArrayHandle validFlags; - validFlags.Allocate(size); + vtkm::cont::ArrayHandle invalidFlags; - invoke(Validate(cellDims, removeAllGhost, removeType, range), ghostField, validFlags); + invoke(Validate(cellDims, removeTypes, range), ghostField, invalidFlags); - vtkm::UInt8 res = vtkm::cont::Algorithm::Reduce(validFlags, vtkm::UInt8(0), vtkm::Maximum()); + vtkm::UInt8 res = vtkm::cont::Algorithm::Reduce(invalidFlags, vtkm::UInt8(0), vtkm::Maximum()); return res == 0; } @@ -237,8 +226,7 @@ template bool CanDoStructuredStrip(const vtkm::cont::UnknownCellSet& cells, const vtkm::cont::ArrayHandle& ghostField, const vtkm::cont::Invoker& invoke, - bool removeAllGhost, - vtkm::UInt8 removeType, + vtkm::UInt8 removeTypes, vtkm::RangeId3& range) { bool canDo = false; @@ -249,9 +237,8 @@ bool CanDoStructuredStrip(const vtkm::cont::UnknownCellSet& cells, auto cells1D = cells.AsCellSet>(); vtkm::Id d = cells1D.GetCellDimensions(); cellDims[0] = d; - vtkm::Id sz = d; - - canDo = CanStrip<1>(ghostField, invoke, removeAllGhost, removeType, range, cellDims, sz); + VTKM_ASSERT(ghostField.GetNumberOfValues() == cellDims[0]); + canDo = CanStrip<1>(ghostField, invoke, removeTypes, range, cellDims); } else if (cells.CanConvert>()) { @@ -259,15 +246,15 @@ bool CanDoStructuredStrip(const vtkm::cont::UnknownCellSet& cells, vtkm::Id2 d = cells2D.GetCellDimensions(); cellDims[0] = d[0]; cellDims[1] = d[1]; - vtkm::Id sz = cellDims[0] * cellDims[1]; - canDo = CanStrip<2>(ghostField, invoke, removeAllGhost, removeType, range, cellDims, sz); + VTKM_ASSERT(ghostField.GetNumberOfValues() == (cellDims[0] * cellDims[1])); + canDo = CanStrip<2>(ghostField, invoke, removeTypes, range, cellDims); } else if (cells.CanConvert>()) { auto cells3D = cells.AsCellSet>(); cellDims = cells3D.GetCellDimensions(); - vtkm::Id sz = cellDims[0] * cellDims[1] * cellDims[2]; - canDo = CanStrip<3>(ghostField, invoke, removeAllGhost, removeType, range, cellDims, sz); + VTKM_ASSERT(ghostField.GetNumberOfValues() == (cellDims[0] * cellDims[1] * cellDims[2])); + canDo = CanStrip<3>(ghostField, invoke, removeTypes, range, cellDims); } return canDo; @@ -330,8 +317,7 @@ VTKM_CONT vtkm::cont::DataSet GhostCellRemove::DoExecute(const vtkm::cont::DataS cells.CanConvert>()) { vtkm::RangeId3 range; - if (CanDoStructuredStrip( - cells, fieldArray, this->Invoke, this->GetRemoveAllGhost(), this->GetRemoveType(), range)) + if (CanDoStructuredStrip(cells, fieldArray, this->Invoke, this->GetTypesToRemove(), range)) { vtkm::filter::entity_extraction::ExtractStructured extract; extract.SetInvoker(this->Invoke); @@ -352,19 +338,8 @@ VTKM_CONT vtkm::cont::DataSet GhostCellRemove::DoExecute(const vtkm::cont::DataS vtkm::cont::UnknownCellSet cellOut; vtkm::worklet::Threshold worklet; - if (this->GetRemoveAllGhost()) - { - cellOut = worklet.Run(cells, fieldArray, field.GetAssociation(), RemoveAllGhosts()); - } - else if (this->GetRemoveByType()) - { - cellOut = worklet.Run( - cells, fieldArray, field.GetAssociation(), RemoveGhostByType(this->GetRemoveType())); - } - else - { - throw vtkm::cont::ErrorFilterExecution("Unsupported ghost cell removal type"); - } + cellOut = worklet.Run( + cells, fieldArray, field.GetAssociation(), RemoveGhostPredicate(this->GetTypesToRemove())); auto mapper = [&](auto& result, const auto& f) { DoMapField(result, f, worklet); }; return this->CreateResult(input, cellOut, mapper); diff --git a/vtkm/filter/entity_extraction/GhostCellRemove.h b/vtkm/filter/entity_extraction/GhostCellRemove.h index 29fcd161f..bd398466f 100644 --- a/vtkm/filter/entity_extraction/GhostCellRemove.h +++ b/vtkm/filter/entity_extraction/GhostCellRemove.h @@ -12,6 +12,7 @@ #define vtk_m_filter_entity_extraction_GhostCellRemove_h #include +#include #include #include @@ -21,47 +22,81 @@ namespace filter { namespace entity_extraction { -/// \brief Removes ghost cells +/// \brief Removes cells marked as ghost cells. +/// +/// This filter inspects the ghost cell field of the input and removes any cells +/// marked as ghost cells. Although this filter nominally operates on ghost cells, +/// other classifications, such as blanked cells, can also be recorded in the ghost +/// cell array. See `vtkm::CellClassification` for the list of flags typical in a +/// ghost array. +/// +/// By default, if the input is a structured data set the filter will attempt to +/// output a structured data set. This will be the case if all the cells along a +/// boundary are marked as ghost cells together, which is common. If creating a +/// structured data set is not possible, an explicit data set is produced. /// class VTKM_FILTER_ENTITY_EXTRACTION_EXPORT GhostCellRemove : public vtkm::filter::FilterField { public: - VTKM_CONT - GhostCellRemove(); + VTKM_CONT GhostCellRemove(); - VTKM_CONT - void RemoveGhostField() { this->RemoveField = true; } - VTKM_CONT - void RemoveAllGhost() { this->RemoveAll = true; } - VTKM_CONT - void RemoveByType(const vtkm::UInt8& vals) - { - this->RemoveAll = false; - this->RemoveVals = vals; - } - VTKM_CONT - bool GetRemoveGhostField() const { return this->RemoveField; } - VTKM_CONT - bool GetRemoveAllGhost() const { return this->RemoveAll; } + /// @brief Specify whether the ghost cell array should be removed from the input. + /// + /// If this flag is true, then the ghost cell array will not be + /// passed to the output. + VTKM_CONT void SetRemoveGhostField(bool flag) { this->RemoveField = flag; } + /// @copydoc SetRemoveGhostField + VTKM_CONT bool GetRemoveGhostField() const { return this->RemoveField; } - VTKM_CONT - bool GetUseGhostCellsAsField() const { return this->UseGhostCellsAsField; } - VTKM_CONT - void SetUseGhostCellsAsField(bool flag) { this->UseGhostCellsAsField = flag; } + /// @brief Specify which types of cells to remove. + /// + /// The types to remove are specified by the flags in `vtkm::CellClassification`. + /// Any cell with a ghost array flag matching one or more of these flags will be removed. + VTKM_CONT void SetTypesToRemove(vtkm::UInt8 typeFlags) { this->TypesToRemove = typeFlags; } + /// @copydoc SetTypesToRemove + VTKM_CONT vtkm::UInt8 GetTypesToRemove() const { return this->TypesToRemove; } - VTKM_CONT - bool GetRemoveByType() const { return !this->RemoveAll; } - VTKM_CONT - vtkm::UInt8 GetRemoveType() const { return this->RemoveVals; } + /// @brief Set filter to remove any special cell type. + /// + /// This method sets the state to remove any cell that does not have a "normal" ghost + /// cell value of 0. Any other value represents a cell that is placeholder or otherwise + /// not really considered part of the cell set. + VTKM_CONT void SetTypesToRemoveToAll() { this->SetTypesToRemove(0xFF); } + /// @brief Returns true if all abnormal cell types are removed. + VTKM_CONT bool AreAllTypesRemoved() const { return this->GetTypesToRemove() == 0xFF; } + + VTKM_DEPRECATED(2.1, "Use SetRemoveGhostField(true).") + VTKM_CONT void RemoveGhostField() { this->SetRemoveGhostField(true); } + VTKM_DEPRECATED(2.1, "Use SetTypesToRemoveToAll().") + VTKM_CONT void RemoveAllGhost() { this->SetTypesToRemoveToAll(); } + VTKM_DEPRECATED(2.1, "Use SetTypesToRemove(vals).") + VTKM_CONT void RemoveByType(const vtkm::UInt8& vals) { this->SetTypesToRemove(vals); } + + VTKM_DEPRECATED(2.1, "Use AreAllTypesRemoved().") + VTKM_CONT bool GetRemoveAllGhost() const { return this->AreAllTypesRemoved(); } + + /// @brief Specify whether the marked ghost cells or a named field should be used as the ghost field. + /// + /// When this flag is true (the default), the filter will get from the input + /// `vtkm::cont::DataSet` the field (with the `GetGhostCellField` method). When + /// this flag is false, the `SetActiveField` method of this class should be used + /// to select which field to use as ghost cells. + VTKM_CONT bool GetUseGhostCellsAsField() const { return this->UseGhostCellsAsField; } + /// @copydoc GetUseGhostCellsAsField + VTKM_CONT void SetUseGhostCellsAsField(bool flag) { this->UseGhostCellsAsField = flag; } + + VTKM_DEPRECATED(2.1, "Use !AreAllTypesRemoved().") + VTKM_CONT bool GetRemoveByType() const { return !this->AreAllTypesRemoved(); } + VTKM_DEPRECATED(2.1, "Use GetTypesToRemove().") + VTKM_CONT vtkm::UInt8 GetRemoveType() const { return this->GetTypesToRemove(); } private: VTKM_CONT vtkm::cont::DataSet DoExecute(const vtkm::cont::DataSet& input) override; bool UseGhostCellsAsField = true; - bool RemoveAll = false; bool RemoveField = false; - vtkm::UInt8 RemoveVals = 0; + vtkm::UInt8 TypesToRemove = 0xFF; }; } // namespace entity_extraction diff --git a/vtkm/filter/entity_extraction/testing/UnitTestGhostCellRemove.cxx b/vtkm/filter/entity_extraction/testing/UnitTestGhostCellRemove.cxx index 42ac24b7f..32107dc80 100644 --- a/vtkm/filter/entity_extraction/testing/UnitTestGhostCellRemove.cxx +++ b/vtkm/filter/entity_extraction/testing/UnitTestGhostCellRemove.cxx @@ -276,12 +276,12 @@ void TestGhostCellRemove() for (auto& rt : removeType) { vtkm::filter::entity_extraction::GhostCellRemove ghostCellRemoval; - ghostCellRemoval.RemoveGhostField(); + ghostCellRemoval.SetRemoveGhostField(true); if (rt == "all") - ghostCellRemoval.RemoveAllGhost(); + ghostCellRemoval.SetTypesToRemoveToAll(); else if (rt == "byType") - ghostCellRemoval.RemoveByType(vtkm::CellClassification::Ghost); + ghostCellRemoval.SetTypesToRemove(vtkm::CellClassification::Ghost); auto output = ghostCellRemoval.Execute(ds); vtkm::Id numCells = output.GetNumberOfCells(); @@ -323,7 +323,7 @@ void TestGhostCellRemove() ds = MakeRectilinear(nx, ny, nz, layer, nameType, true); vtkm::filter::entity_extraction::GhostCellRemove ghostCellRemoval; - ghostCellRemoval.RemoveGhostField(); + ghostCellRemoval.SetRemoveGhostField(true); auto output = ghostCellRemoval.Execute(ds); VTKM_TEST_ASSERT(output.GetCellSet().IsType>(), "Wrong cell type for explicit conversion");