Merge topic 'correct_improper_cellset_indexing'

fa9ffac7c Correct improper cellset indexing
b0c6e18ea DataSet queries for cellset and coordinate index won't trow

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Kenneth Moreland <kmorel@sandia.gov>
Merge-request: !1777

docs/changelog/dataset-no-throw-on-index-queries.md

@@ -0,0 +1,11 @@
+# DataSet queries for CellSet and Coordinate System Indices don't throw
+
+Asking for the index of a `vtkm::cont::CellSet` or `vtkm::cont::CoordinateSystem` by
+name now returns a `-1` when no matching item has been found instead of throwing
+an exception.
+
+This was done to make the interface of `vtkm::cont::DataSet` to follow the guideline
+"Only unrepresentable things should raise exceptions". The index of a non-existent item
+is representable by `-1` and therefore we shouldn't throw, like wise the methods that return
+references can still throw exceptions as you can't have a reference to an non-existent item.
+

docs/changelog/filter-field-smarter-active-cellset-selection.md

@@ -0,0 +1,6 @@
+# FilterField now tries to be smart when selecting active cellset
+
+Now when a calls a `vtkm::filter::FilterField` algorithm without an explicit
+active CellSet, if the field is cell based we set the active `vtkm::cont::CellSet`
+to be the one associated with that field. If that `vtkm::cont::CellSet` doesn't
+exist we default back to using the first CellSet in the input `vtkm::cont::DataSet`.

docs/changelog/filters-removal-of-filter-cell.md

@@ -0,0 +1,9 @@
+# FilterField now provides all functionality of FilterCell
+
+The FilterCell was a subclass of `vtkm::filter::FilterField` and behaves essentially the same
+but provided the pair of methods `SetActiveCellSetIndex` and `GetActiveCellSetIndex`.
+It was a common misconception that `FilterCell` was meant for Cell based algorithms, instead of
+algorithms that required access to the active `vtkm::cont::CellSet`.
+
+By moving `SetActiveCellSetIndex` and `GetActiveCellSetIndex` to FilterField, we remove this confusion.
+

vtkm/cont/DataSet.cxx

@@ -55,30 +55,67 @@ const vtkm::cont::CoordinateSystem& DataSet::GetCoordinateSystem(vtkm::Id index)
 
 vtkm::Id DataSet::GetCoordinateSystemIndex(const std::string& name) const
 {
-  bool found;
-  vtkm::Id index = this->FindCoordinateSystemIndex(name, found);
-  if (found)
+  vtkm::Id index = -1;
+  for (auto i = this->CoordSystems.begin(); i != this->CoordSystems.end(); ++i)
   {
-    return index;
+    if (i->GetName() == name)
+    {
+      index = static_cast<vtkm::Id>(std::distance(this->CoordSystems.begin(), i));
+      break;
+    }
   }
-  else
+  return index;
+}
+
+const vtkm::cont::CoordinateSystem& DataSet::GetCoordinateSystem(const std::string& name) const
+{
+  vtkm::Id index = this->GetCoordinateSystemIndex(name);
+  if (index < 0)
   {
-    throw vtkm::cont::ErrorBadValue("No coordinate system with requested name");
+    std::string error_message("No coordinate system with the name " + name +
+                              " valid names are: \n");
+    for (const auto& cs : this->CoordSystems)
+    {
+      error_message += cs.GetName() + "\n";
+    }
+    throw vtkm::cont::ErrorBadValue(error_message);
   }
+  return this->GetCoordinateSystem(index);
+}
+
+const vtkm::cont::DynamicCellSet& DataSet::GetCellSet(vtkm::Id index) const
+{
+  VTKM_ASSERT((index >= 0) && (index < this->GetNumberOfCellSets()));
+  return this->CellSets[static_cast<std::size_t>(index)];
 }
 
 vtkm::Id DataSet::GetCellSetIndex(const std::string& name) const
 {
-  bool found;
-  vtkm::Id index = this->FindCellSetIndex(name, found);
-  if (found)
+  vtkm::Id index = -1;
+  for (auto i = this->CellSets.begin(); i != this->CellSets.end(); ++i)
   {
-    return index;
+    if (i->GetName() == name)
+    {
+      index = static_cast<vtkm::Id>(std::distance(this->CellSets.begin(), i));
+      break;
+    }
   }
-  else
+  return index;
+}
+
+const vtkm::cont::DynamicCellSet& DataSet::GetCellSet(const std::string& name) const
+{
+  vtkm::Id index = this->GetCellSetIndex(name);
+  if (index < 0)
   {
-    throw vtkm::cont::ErrorBadValue("No cell set with requested name");
+    std::string error_message("No cell set with the name " + name + " valid names are: \n");
+    for (const auto& cs : this->CellSets)
+    {
+      error_message += cs.GetName() + "\n";
+    }
+    throw vtkm::cont::ErrorBadValue(error_message);
   }
+  return this->GetCellSet(index);
 }
 
 void DataSet::PrintSummary(std::ostream& out) const
@@ -121,34 +158,5 @@ vtkm::Id DataSet::FindFieldIndex(const std::string& name,
   return -1;
 }
 
-
-vtkm::Id DataSet::FindCoordinateSystemIndex(const std::string& name, bool& found) const
-{
-  for (std::size_t index = 0; index < this->CoordSystems.size(); ++index)
-  {
-    if (this->CoordSystems[index].GetName() == name)
-    {
-      found = true;
-      return static_cast<vtkm::Id>(index);
-    }
-  }
-  found = false;
-  return -1;
-}
-
-vtkm::Id DataSet::FindCellSetIndex(const std::string& name, bool& found) const
-{
-  for (std::size_t index = 0; index < static_cast<size_t>(this->GetNumberOfCellSets()); ++index)
-  {
-    if (this->CellSets[index].GetName() == name)
-    {
-      found = true;
-      return static_cast<vtkm::Id>(index);
-    }
-  }
-  found = false;
-  return -1;
-}
-
 } // namespace cont
 } // namespace vtkm

vtkm/cont/DataSet.h

@@ -60,11 +60,16 @@ public:
     return found;
   }
 
+
+  /// Returns the first field that matches the provided name and association
+  /// Will return -1 if no match is found
   VTKM_CONT
   vtkm::Id GetFieldIndex(
     const std::string& name,
     vtkm::cont::Field::Association assoc = vtkm::cont::Field::Association::ANY) const;
 
+  /// Returns the first field that matches the provided name and association
+  /// Will throw an exception if no match is found
   VTKM_CONT
   const vtkm::cont::Field& GetField(
     const std::string& name,
@@ -73,12 +78,16 @@ public:
     return this->GetField(this->GetFieldIndex(name, assoc));
   }
 
+  /// Returns the first cell field that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
   const vtkm::cont::Field& GetCellField(const std::string& name) const
   {
     return this->GetField(name, vtkm::cont::Field::Association::CELL_SET);
   }
 
+  /// Returns the first point field that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
   const vtkm::cont::Field& GetPointField(const std::string& name) const
   {
@@ -91,25 +100,25 @@ public:
     this->CoordSystems.push_back(cs);
   }
 
-  VTKM_CONT
-  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(vtkm::Id index = 0) const;
-
   VTKM_CONT
   bool HasCoordinateSystem(const std::string& name) const
   {
-    bool found;
-    this->FindCoordinateSystemIndex(name, found);
-    return found;
+    return this->GetCoordinateSystemIndex(name) >= 0;
   }
 
+  VTKM_CONT
+  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(vtkm::Id index = 0) const;
+
+  /// Returns the index for the first CoordinateSystem whose
+  /// name matches the provided string.
+  /// Will return -1 if no match is found
   VTKM_CONT
   vtkm::Id GetCoordinateSystemIndex(const std::string& name) const;
 
+  /// Returns the first CoordinateSystem that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
-  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(const std::string& name) const
-  {
-    return this->GetCoordinateSystem(this->GetCoordinateSystemIndex(name));
-  }
+  const vtkm::cont::CoordinateSystem& GetCoordinateSystem(const std::string& name) const;
 
   VTKM_CONT
   void AddCellSet(const vtkm::cont::DynamicCellSet& cellSet) { this->CellSets.push_back(cellSet); }
@@ -122,28 +131,24 @@ public:
   }
 
   VTKM_CONT
-  vtkm::cont::DynamicCellSet GetCellSet(vtkm::Id index = 0) const
-  {
-    VTKM_ASSERT((index >= 0) && (index < this->GetNumberOfCellSets()));
-    return this->CellSets[static_cast<std::size_t>(index)];
-  }
+  bool HasCellSet(const std::string& name) const { return this->GetCellSetIndex(name) >= 0; }
 
   VTKM_CONT
-  bool HasCellSet(const std::string& name) const
-  {
-    bool found;
-    this->FindCellSetIndex(name, found);
-    return found;
-  }
+  const vtkm::cont::DynamicCellSet& GetCellSet(vtkm::Id index = 0) const;
 
+
+  /// Returns the index for the first cell set whose
+  /// name matches the provided string.
+  /// Will return -1 if no match is found
   VTKM_CONT
   vtkm::Id GetCellSetIndex(const std::string& name) const;
 
+
+  /// Returns the first DynamicCellSet that matches the provided name.
+  /// Will throw an exception if no match is found
   VTKM_CONT
-  vtkm::cont::DynamicCellSet GetCellSet(const std::string& name) const
-  {
-    return this->GetCellSet(this->GetCellSetIndex(name));
-  }
+  const vtkm::cont::DynamicCellSet& GetCellSet(const std::string& name) const;
+
 
   VTKM_CONT
   vtkm::IdComponent GetNumberOfCellSets() const
@@ -180,12 +185,6 @@ private:
   vtkm::Id FindFieldIndex(const std::string& name,
                           vtkm::cont::Field::Association association,
                           bool& found) const;
-
-  VTKM_CONT
-  vtkm::Id FindCoordinateSystemIndex(const std::string& name, bool& found) const;
-
-  VTKM_CONT
-  vtkm::Id FindCellSetIndex(const std::string& name, bool& found) const;
 };
 
 } // namespace cont

vtkm/filter/CMakeLists.txt

@@ -98,7 +98,6 @@ set(header_template_sources
   ExtractStructured.hxx
   FieldToColors.hxx
   Filter.hxx
-  FilterCell.hxx
   FilterDataSet.hxx
   FilterDataSetWithField.hxx
   FilterField.hxx

vtkm/filter/ContourTreeUniformAugmented.hxx

@@ -115,7 +115,7 @@ vtkm::cont::DataSet ContourTreePPP2::DoExecute(const vtkm::cont::DataSet& input,
   vtkm::Id nRows;
   vtkm::Id nCols;
   vtkm::Id nSlices = 1;
-  const auto& cells = input.GetCellSet(this->GetActiveCoordinateSystemIndex());
+  const auto& cells = input.GetCellSet(this->GetActiveCellSetIndex());
   vtkm::filter::ApplyPolicy(cells, policy).CastAndCall(GetRowsColsSlices(), nRows, nCols, nSlices);
 
   // Run the worklet

vtkm/filter/FilterCell.h

@@ -19,27 +19,7 @@ namespace filter
 {
 
 template <class Derived>
-class FilterCell : public vtkm::filter::FilterField<Derived>
-{
-public:
-  VTKM_CONT
-  FilterCell();
-
-  VTKM_CONT
-  ~FilterCell();
-
-  VTKM_CONT
-  void SetActiveCellSetIndex(vtkm::Id index) { this->CellSetIndex = index; }
-
-  VTKM_CONT
-  vtkm::Id GetActiveCellSetIndex() const { return this->CellSetIndex; }
-
-protected:
-  vtkm::Id CellSetIndex;
-};
+using FilterCell = vtkm::filter::FilterField<Derived>;
+}
 }
-} // namespace vtkm::filter
-
-#include <vtkm/filter/FilterCell.hxx>
-
 #endif // vtk_m_filter_CellFilter_h

vtkm/filter/FilterCell.hxx

@@ -1,30 +0,0 @@
-//============================================================================
-//  Copyright (c) Kitware, Inc.
-//  All rights reserved.
-//  See LICENSE.txt for details.
-//
-//  This software is distributed WITHOUT ANY WARRANTY; without even
-//  the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
-//  PURPOSE.  See the above copyright notice for more information.
-//============================================================================
-
-namespace vtkm
-{
-namespace filter
-{
-
-//----------------------------------------------------------------------------
-template <class Derived>
-inline VTKM_CONT FilterCell<Derived>::FilterCell()
-  : vtkm::filter::FilterField<Derived>()
-  , CellSetIndex(0)
-{
-}
-
-//----------------------------------------------------------------------------
-template <class Derived>
-inline VTKM_CONT FilterCell<Derived>::~FilterCell()
-{
-}
-}
-}

vtkm/filter/FilterField.h

@@ -70,15 +70,30 @@ public:
 
 
   //@{
-  /// Select the coordinate system index to make active to use as the field when
-  /// UseCoordinateSystemAsField is true.
+  /// Select the coordinate system index to make active to use when processing the input
+  /// DataSet. This is used primarily by the Filter to select the coordinate system
+  /// to use as a field when \c UseCoordinateSystemAsField is true.
   VTKM_CONT
-  void SetActiveCoordinateSystem(vtkm::Id index) { this->CoordinateSystemIndex = index; }
+  void SetActiveCoordinateSystem(vtkm::Id index)
+  {
+    this->DeduceCellSetIndex = false;
+    this->CoordinateSystemIndex = index;
+  }
 
   VTKM_CONT
   vtkm::Id GetActiveCoordinateSystemIndex() const { return this->CoordinateSystemIndex; }
   //@}
 
+  //@{
+  /// Override the cellSet index to be active when executing the filter. By default
+  /// When processing cell fields the active cellset is the one assoicated with the
+  /// provided field, otherwise it will default to 0.
+  VTKM_CONT
+  void SetActiveCellSetIndex(vtkm::Id index) { this->CellSetIndex = index; }
+
+  VTKM_CONT
+  vtkm::Id GetActiveCellSetIndex() const { return this->CellSetIndex; }
+
   /// These are provided to satisfy the Filter API requirements.
   template <typename DerivedPolicy>
   VTKM_CONT vtkm::cont::DataSet PrepareForExecution(
@@ -97,11 +112,14 @@ public:
     const vtkm::cont::CoordinateSystem& field,
     const vtkm::filter::PolicyBase<DerivedPolicy>& policy);
 
+protected:
 private:
   std::string OutputFieldName;
+  vtkm::Id CellSetIndex;
   vtkm::Id CoordinateSystemIndex;
   std::string ActiveFieldName;
   vtkm::cont::Field::Association ActiveFieldAssociation;
+  bool DeduceCellSetIndex;
   bool UseCoordinateSystemAsField;
 
   friend class vtkm::filter::Filter<Derived>;

vtkm/filter/FilterField.hxx

@@ -30,9 +30,11 @@ namespace filter
 template <typename Derived>
 inline VTKM_CONT FilterField<Derived>::FilterField()
   : OutputFieldName()
+  , CellSetIndex(0)
   , CoordinateSystemIndex(0)
   , ActiveFieldName()
   , ActiveFieldAssociation(vtkm::cont::Field::Association::ANY)
+  , DeduceCellSetIndex(true)
   , UseCoordinateSystemAsField(false)
 {
 }
@@ -72,6 +74,17 @@ inline VTKM_CONT vtkm::cont::DataSet FilterField<Derived>::PrepareForExecution(
   const vtkm::cont::Field& field,
   const vtkm::filter::PolicyBase<DerivedPolicy>& policy)
 {
+
+  //If the user hasn't specified a field, try and deduce
+  //one based on the field. Once we are done executing we
+  //need to rollback the CellSetIndex so this isn't used on
+  //a subsequent execution by a non-cell field
+  if (this->DeduceCellSetIndex && field.IsFieldCell())
+  {
+    //If no cellset is found that matches the name, default to the first
+    //cellset
+    this->CellSetIndex = std::max(vtkm::Id{ 0 }, input.GetCellSetIndex(field.GetAssocCellSet()));
+  }
   vtkm::filter::FieldMetadata metaData(field);
   vtkm::cont::DataSet result;
 
@@ -83,6 +96,10 @@ inline VTKM_CONT vtkm::cont::DataSet FilterField<Derived>::PrepareForExecution(
     metaData,
     policy,
     result);
+  if (this->DeduceCellSetIndex)
+  {
+    this->CellSetIndex = 0;
+  }
   return result;
 }
 
@@ -96,7 +113,6 @@ inline VTKM_CONT vtkm::cont::DataSet FilterField<Derived>::PrepareForExecution(
 {
   //We have a special signature just for CoordinateSystem, so that we can ask
   //the policy for the storage types and value types just for coordinate systems
-
   vtkm::filter::FieldMetadata metaData(field);
   vtkm::cont::DataSet result;
 

vtkm/filter/Lagrangian.hxx

@@ -228,8 +228,7 @@ inline VTKM_CONT vtkm::cont::DataSet Lagrangian::DoExecute(
 
   cycle += 1;
   std::cout << "Cycle : " << cycle << std::endl;
-  const vtkm::cont::DynamicCellSet& cells =
-    input.GetCellSet(this->GetActiveCoordinateSystemIndex());
+  const vtkm::cont::DynamicCellSet& cells = input.GetCellSet(this->GetActiveCellSetIndex());
   const vtkm::cont::CoordinateSystem& coords =
     input.GetCoordinateSystem(this->GetActiveCoordinateSystemIndex());
   vtkm::Bounds bounds = input.GetCoordinateSystem().GetBounds();

vtkm/filter/ZFPCompressor2D.hxx

@@ -61,7 +61,7 @@ inline VTKM_CONT vtkm::cont::DataSet ZFPCompressor2D::DoExecute(
     }
   }
   vtkm::cont::CellSetStructured<2> cellSet;
-  input.GetCellSet(this->GetActiveCoordinateSystemIndex()).CopyTo(cellSet);
+  input.GetCellSet(this->GetActiveCellSetIndex()).CopyTo(cellSet);
   vtkm::Id2 pointDimensions = cellSet.GetPointDimensions();
 
 

vtkm/filter/ZFPCompressor3D.hxx

@@ -61,7 +61,7 @@ inline VTKM_CONT vtkm::cont::DataSet ZFPCompressor3D::DoExecute(
     }
   }
   vtkm::cont::CellSetStructured<3> cellSet;
-  input.GetCellSet(this->GetActiveCoordinateSystemIndex()).CopyTo(cellSet);
+  input.GetCellSet(this->GetActiveCellSetIndex()).CopyTo(cellSet);
   vtkm::Id3 pointDimensions = cellSet.GetPointDimensions();
 
 

vtkm/filter/ZFPDecompressor2D.hxx

@@ -60,7 +60,7 @@ inline VTKM_CONT vtkm::cont::DataSet ZFPDecompressor2D::DoExecute(
   }
 
   vtkm::cont::CellSetStructured<2> cellSet;
-  input.GetCellSet(this->GetActiveCoordinateSystemIndex()).CopyTo(cellSet);
+  input.GetCellSet(this->GetActiveCellSetIndex()).CopyTo(cellSet);
   vtkm::Id2 pointDimensions = cellSet.GetPointDimensions();
 
   vtkm::cont::ArrayHandle<vtkm::Float64> decompress;

vtkm/worklet/Contour.h

@@ -808,7 +808,7 @@ private:
     vtkm::cont::ArrayHandle<vtkm::Vec<NormalType, 3>, StorageTagNormals> normals,
     bool withNormals)
   {
-    vtkm::cont::CellSetSingleType<> outputCells("contour");
+    vtkm::cont::CellSetSingleType<> outputCells(cells.GetName());
 
     DeduceCellType<ValueType,
                    CoordinateSystem,
@@ -949,7 +949,7 @@ private:
       this->InterpolationEdgeIds, this->InterpolationWeights, coordinateSystem, vertices);
 
     //assign the connectivity to the cell set
-    vtkm::cont::CellSetSingleType<> outputCells("contour");
+    vtkm::cont::CellSetSingleType<> outputCells(cells.GetName());
     outputCells.Fill(vertices.GetNumberOfValues(), vtkm::CELL_SHAPE_TRIANGLE, 3, connectivity);
 
     //now that the vertices have been generated we can generate the normals