From 63702d5d13f7567c2114476875311b579a10c11c Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Mon, 31 Oct 2022 11:52:38 -0600 Subject: [PATCH] Check to make sure that the fields in a DataSet are the proper length It is possible in a `DataSet` to add a point field (or coordinate system) that has a different number of points than reported in the cell set. Likewise for the number of cells in cell fields. This is very bad practice because it is likely to lead to crashes in worklets that are expecting arrays of an appropriate length. Although `DataSet` will still allow this, a warning will be added to the VTK-m logging to alert users of the inconsistency introduced into the `DataSet`. Since warnings are by default printed to standard error, users are likely to see it. --- docs/changelog/field-length-checking.md | 12 +++ vtkm/cont/DataSet.cxx | 97 ++++++++++++++++++- vtkm/cont/DataSet.h | 11 +-- .../UnitTestDataSetConvertToExpected.cxx | 5 + vtkm/cont/testlib/MakeTestDataSet.cxx | 4 +- 5 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 docs/changelog/field-length-checking.md diff --git a/docs/changelog/field-length-checking.md b/docs/changelog/field-length-checking.md new file mode 100644 index 000000000..e8624ed26 --- /dev/null +++ b/docs/changelog/field-length-checking.md @@ -0,0 +1,12 @@ +# Check to make sure that the fields in a DataSet are the proper length + +It is possible in a `DataSet` to add a point field (or coordinate system) +that has a different number of points than reported in the cell set. +Likewise for the number of cells in cell fields. This is very bad practice +because it is likely to lead to crashes in worklets that are expecting +arrays of an appropriate length. + +Although `DataSet` will still allow this, a warning will be added to the +VTK-m logging to alert users of the inconsistency introduced into the +`DataSet`. Since warnings are by default printed to standard error, users +are likely to see it. diff --git a/vtkm/cont/DataSet.cxx b/vtkm/cont/DataSet.cxx index c4ee2b731..8e18a83fc 100644 --- a/vtkm/cont/DataSet.cxx +++ b/vtkm/cont/DataSet.cxx @@ -9,6 +9,57 @@ //============================================================================ #include +#include + +namespace +{ + +VTKM_CONT void CheckFieldSize(const vtkm::cont::UnknownCellSet& cellSet, + const vtkm::cont::Field& field) +{ + if (!cellSet.IsValid()) + { + return; + } + switch (field.GetAssociation()) + { + case vtkm::cont::Field::Association::Points: + if (cellSet.GetNumberOfPoints() != field.GetData().GetNumberOfValues()) + { + VTKM_LOG_S(vtkm::cont::LogLevel::Warn, + "The size of field `" + << field.GetName() << "` (" << field.GetData().GetNumberOfValues() + << " values) does not match the size of the data set structure (" + << cellSet.GetNumberOfPoints() << " points)."); + } + break; + case vtkm::cont::Field::Association::Cells: + if (cellSet.GetNumberOfCells() != field.GetData().GetNumberOfValues()) + { + VTKM_LOG_S(vtkm::cont::LogLevel::Warn, + "The size of field `" + << field.GetName() << "` (" << field.GetData().GetNumberOfValues() + << " values) does not match the size of the data set structure (" + << cellSet.GetNumberOfCells() << " cells)."); + } + break; + default: + // Ignore as the association does not match any topological element. + break; + } +} + +VTKM_CONT void CheckFieldSizes(const vtkm::cont::UnknownCellSet& cellSet, + const vtkm::cont::internal::FieldCollection& fields) +{ + vtkm::IdComponent numFields = fields.GetNumberOfFields(); + for (vtkm::IdComponent fieldIndex = 0; fieldIndex < numFields; ++fieldIndex) + { + CheckFieldSize(cellSet, fields.GetField(fieldIndex)); + } +} + +} // anonymous namespace namespace vtkm { @@ -38,6 +89,12 @@ void DataSet::Clear() this->CellSet = this->CellSet.NewInstance(); } +void DataSet::AddField(const Field& field) +{ + CheckFieldSize(this->CellSet, field); + this->Fields.AddField(field); +} + vtkm::Id DataSet::GetNumberOfCells() const { return this->CellSet.GetNumberOfCells(); @@ -45,11 +102,43 @@ vtkm::Id DataSet::GetNumberOfCells() const vtkm::Id DataSet::GetNumberOfPoints() const { - if (this->CoordSystems.empty()) + if (this->CellSet.IsValid()) { - return 0; + return this->CellSet.GetNumberOfPoints(); } - return this->CoordSystems[0].GetNumberOfPoints(); + + // If there is no cell set, then try to use a coordinate system to get the number + // of points. + if (this->GetNumberOfCoordinateSystems() > 0) + { + return this->GetCoordinateSystem().GetNumberOfPoints(); + } + + // If there is no coordinate system either, we can try to guess the number of + // points by finding a point field. + for (vtkm::IdComponent fieldIdx = 0; fieldIdx < this->Fields.GetNumberOfFields(); ++fieldIdx) + { + const vtkm::cont::Field& field = this->Fields.GetField(fieldIdx); + if (field.GetAssociation() == vtkm::cont::Field::Association::Points) + { + return field.GetData().GetNumberOfValues(); + } + } + + // There are no point fields either. + return 0; +} + +void DataSet::AddCoordinateSystem(const vtkm::cont::CoordinateSystem& cs) +{ + CheckFieldSize(this->CellSet, cs); + this->CoordSystems.push_back(cs); +} + +void DataSet::SetCellSetImpl(const vtkm::cont::UnknownCellSet& cellSet) +{ + CheckFieldSizes(cellSet, this->Fields); + this->CellSet = cellSet; } void DataSet::CopyStructure(const vtkm::cont::DataSet& source) @@ -57,6 +146,8 @@ void DataSet::CopyStructure(const vtkm::cont::DataSet& source) this->CoordSystems = source.CoordSystems; this->CellSet = source.CellSet; this->GhostCellName = source.GhostCellName; + + CheckFieldSizes(this->CellSet, this->Fields); } const vtkm::cont::CoordinateSystem& DataSet::GetCoordinateSystem(vtkm::Id index) const diff --git a/vtkm/cont/DataSet.h b/vtkm/cont/DataSet.h index 8dfe542b6..486194389 100644 --- a/vtkm/cont/DataSet.h +++ b/vtkm/cont/DataSet.h @@ -55,7 +55,7 @@ public: /// to have the same number of points. VTKM_CONT vtkm::Id GetNumberOfPoints() const; - VTKM_CONT void AddField(const Field& field) { this->Fields.AddField(field); } + VTKM_CONT void AddField(const Field& field); VTKM_CONT const vtkm::cont::Field& GetField(vtkm::Id index) const { return this->Fields.GetField(index); } @@ -265,10 +265,7 @@ public: VTKM_CONT - void AddCoordinateSystem(const vtkm::cont::CoordinateSystem& cs) - { - this->CoordSystems.push_back(cs); - } + void AddCoordinateSystem(const vtkm::cont::CoordinateSystem& cs); VTKM_CONT bool HasCoordinateSystem(const std::string& name) const @@ -310,7 +307,7 @@ public: VTKM_CONT void SetCellSet(const CellSetType& cellSet) { VTKM_IS_KNOWN_OR_UNKNOWN_CELL_SET(CellSetType); - this->CellSet = vtkm::cont::UnknownCellSet(cellSet); + this->SetCellSetImpl(cellSet); } VTKM_CONT @@ -359,6 +356,8 @@ private: vtkm::cont::UnknownCellSet CellSet; std::shared_ptr GhostCellName; + + VTKM_CONT void SetCellSetImpl(const vtkm::cont::UnknownCellSet& cellSet); }; } // namespace cont diff --git a/vtkm/cont/testing/UnitTestDataSetConvertToExpected.cxx b/vtkm/cont/testing/UnitTestDataSetConvertToExpected.cxx index 58b94268a..dc6717a70 100644 --- a/vtkm/cont/testing/UnitTestDataSetConvertToExpected.cxx +++ b/vtkm/cont/testing/UnitTestDataSetConvertToExpected.cxx @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -61,6 +62,10 @@ vtkm::cont::DataSet MakeDataSet() vtkm::cont::DataSet dataset; + vtkm::cont::CellSetStructured<3> cellSet; + cellSet.SetPointDimensions(vtkm::Id3(DIM_SIZE)); + dataset.SetCellSet(cellSet); + dataset.AddCoordinateSystem(vtkm::cont::CoordinateSystem("coords", MakeCoordinates())); dataset.AddPointField("scalars", MakeField()); dataset.AddPointField("vectors", MakeVecField()); diff --git a/vtkm/cont/testlib/MakeTestDataSet.cxx b/vtkm/cont/testlib/MakeTestDataSet.cxx index 76d01fefd..bdfee8762 100644 --- a/vtkm/cont/testlib/MakeTestDataSet.cxx +++ b/vtkm/cont/testlib/MakeTestDataSet.cxx @@ -1302,7 +1302,7 @@ vtkm::cont::DataSet MakeTestDataSet::Make3DExplicitDataSet8() // Coordinates const int nVerts = 8; - const int nCells = 10; + const int nCells = 9; using CoordType = vtkm::Vec3f_32; std::vector coords = { { -0.707f, -0.354f, -0.354f }, { 0.000f, -0.854f, 0.146f }, { 0.000f, -0.146f, 0.854f }, { -0.707f, 0.354f, 0.354f }, @@ -1366,7 +1366,7 @@ vtkm::cont::DataSet MakeTestDataSet::Make3DExplicitDataSet8() // Field data vtkm::Float32 pointvar[nVerts] = { 100.0f, 78.0f, 49.0f, 17.0f, 94.0f, 71.0f, 47.0f, 57.0f }; - vtkm::Float32 cellvar[nCells] = { 0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f, 9.0f }; + vtkm::Float32 cellvar[nCells] = { 0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f, 7.0f, 8.0f }; dataSet.AddPointField("pointvar", pointvar, nVerts); dataSet.AddCellField("cellvar", cellvar, nCells);