Merge topic 'resize-extracted-components'

23469cab6 Add ability to resize ArrayHandleRecombineVec
2061e95ef Add ability to resize ArrayHandleStride

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Li-Ta Lo <ollie@lanl.gov>
Merge-request: !2964

docs/changelog/resize-extracted-component.md

@@ -0,0 +1,18 @@
+# Added ability to resize strided arrays from ArrayExtractComponent
+
+Previously, it was not possible to resize an `ArrayHandleStride` because
+the operation is a bit ambiguous. The actual array is likely to be padded
+by some amount, and there could be an unknown amount of space skipped at
+the beginning.
+
+However, there is a good reason to want to resize `ArrayHandleStride`. This
+is the array used to implement the `ArrayExtractComponent` feature, and
+this in turn is used when extracting arrays from an `UnknownArrayHandle`
+whether independent or as an `ArrayHandleRecombineVec`.
+
+The problem really happens when you create an array of an unknown type in
+an `UnknownArrayHandle` (such as with `NewInstance`) and then use that as
+an output to a worklet. Sure, you could use `ArrayHandle::Allocate` to
+resize before getting the array, but that is awkward for programers.
+Instead, allow the extracted arrays to be resized as normal output arrays
+would be.

vtkm/cont/ArrayHandleRecombineVec.h

@@ -24,12 +24,18 @@ namespace vtkm
 namespace internal
 {
 
+// Forward declaration
+template <typename SourcePortalType>
+class ArrayPortalRecombineVec;
+
 template <typename PortalType>
 class RecombineVec
 {
   vtkm::VecCConst<PortalType> Portals;
   vtkm::Id Index;
 
+  friend vtkm::internal::ArrayPortalRecombineVec<PortalType>;
+
 public:
   using ComponentType = typename std::remove_const<typename PortalType::ValueType>::type;
 
@@ -320,13 +326,26 @@ public:
 
   VTKM_EXEC_CONT void Set(vtkm::Id index, const ValueType& value) const
   {
-    // The ValueType is actually a reference back to the portals, and sets to it should
-    // already be set in the portal. Thus, we don't really need to do anything.
-    VTKM_ASSERT(value.GetIndex() == index);
+    if ((value.GetIndex() == index) && (value.Portals.GetPointer() == this->Portals))
+    {
+      // The ValueType is actually a reference back to the portals. If this reference is
+      // actually pointing back to the same index, we don't need to do anything.
+    }
+    else
+    {
+      this->DoCopy(index, value);
+    }
   }
 
   template <typename T>
   VTKM_EXEC_CONT void Set(vtkm::Id index, const T& value) const
+  {
+    this->DoCopy(index, value);
+  }
+
+private:
+  template <typename T>
+  VTKM_EXEC_CONT void DoCopy(vtkm::Id index, const T& value) const
   {
     using Traits = vtkm::VecTraits<T>;
     VTKM_ASSERT(Traits::GetNumberOfComponents(value) == this->NumberOfComponents);
@@ -408,8 +427,6 @@ class Storage<vtkm::internal::RecombineVec<ReadWritePortal>,
   }
 
 public:
-  VTKM_STORAGE_NO_RESIZE;
-
   using ReadPortalType = vtkm::internal::ArrayPortalRecombineVec<ReadWritePortal>;
   using WritePortalType = vtkm::internal::ArrayPortalRecombineVec<ReadWritePortal>;
 
@@ -426,6 +443,19 @@ public:
     return SourceStorage::GetNumberOfValues(BuffersForComponent(buffers, 0));
   }
 
+  VTKM_CONT static void ResizeBuffers(vtkm::Id numValues,
+                                      const std::vector<vtkm::cont::internal::Buffer>& buffers,
+                                      vtkm::CopyFlag preserve,
+                                      vtkm::cont::Token& token)
+  {
+    vtkm::IdComponent numComponents = NumberOfComponents(buffers);
+    for (vtkm::IdComponent component = 0; component < numComponents; ++component)
+    {
+      SourceStorage::ResizeBuffers(
+        numValues, BuffersForComponent(buffers, component), preserve, token);
+    }
+  }
+
   VTKM_CONT static void Fill(const std::vector<vtkm::cont::internal::Buffer>&,
                              const vtkm::internal::RecombineVec<ReadWritePortal>&,
                              vtkm::Id,

vtkm/cont/ArrayHandleStride.h

@@ -158,8 +158,6 @@ class VTKM_ALWAYS_EXPORT Storage<T, vtkm::cont::StorageTagStride>
   using StrideInfo = vtkm::internal::ArrayStrideInfo;
 
 public:
-  VTKM_STORAGE_NO_RESIZE;
-
   using ReadPortalType = vtkm::internal::ArrayPortalStrideRead<T>;
   using WritePortalType = vtkm::internal::ArrayPortalStrideWrite<T>;
 
@@ -174,6 +172,85 @@ public:
     return GetInfo(buffers).NumberOfValues;
   }
 
+  VTKM_CONT static void ResizeBuffers(vtkm::Id numValues,
+                                      const std::vector<vtkm::cont::internal::Buffer>& buffers,
+                                      vtkm::CopyFlag preserve,
+                                      vtkm::cont::Token& token)
+  {
+    StrideInfo& info = GetInfo(buffers);
+
+    if (info.NumberOfValues == numValues)
+    {
+      // Array resized to current size. Don't need to do anything.
+      return;
+    }
+
+    // Find the end index after dealing with the divsor and modulo.
+    auto lengthDivMod = [info](vtkm::Id length) -> vtkm::Id {
+      vtkm::Id resultLength = ((length - 1) / info.Divisor) + 1;
+      if ((info.Modulo > 0) && (info.Modulo < resultLength))
+      {
+        resultLength = info.Modulo;
+      }
+      return resultLength;
+    };
+    vtkm::Id lastStridedIndex = lengthDivMod(numValues);
+
+    vtkm::Id originalStride;
+    vtkm::Id originalOffset;
+    if (info.Stride > 0)
+    {
+      originalStride = info.Stride;
+      originalOffset = info.Offset;
+    }
+    else
+    {
+      // The stride is negative, which means we are counting backward. Here we have to be careful
+      // about the offset, which should move to push to the end of the array. We also need to
+      // be careful about multiplying by the stride.
+      originalStride = -info.Stride;
+
+      vtkm::Id originalSize = lengthDivMod(info.NumberOfValues);
+
+      // Because the stride is negative, we expect the offset to be at the end of the array.
+      // We will call the "real" offset the distance from that end.
+      originalOffset = originalSize - info.Offset - 1;
+    }
+
+    // If the offset is more than the stride, that means there are values skipped at the
+    // beginning of the array, and it is impossible to know exactly how many. In this case,
+    // we cannot know how to resize. (If this is an issue, we will have to change
+    // `ArrayHandleStride` to take resizing parameters.)
+    if (originalOffset >= originalStride)
+    {
+      if (numValues == 0)
+      {
+        // Array resized to zero. This can happen when releasing resources.
+        // Should we try to clear out the buffers, or avoid that for messing up shared buffers?
+        return;
+      }
+      throw vtkm::cont::ErrorBadAllocation(
+        "Cannot resize stride array with offset greater than stride (start of stride unknown).");
+    }
+
+    // lastIndex should be the index in the source array after each stride block. Assuming the
+    // offset is inside the first stride, this should be the end of the array regardless of
+    // offset.
+    vtkm::Id lastIndex = lastStridedIndex * originalStride;
+
+    buffers[1].SetNumberOfBytes(
+      vtkm::internal::NumberOfValuesToNumberOfBytes<T>(lastIndex), preserve, token);
+    info.NumberOfValues = numValues;
+
+    if (info.Stride < 0)
+    {
+      // As described above, when the stride is negative, we are counting backward. This means
+      // that the offset is actually relative to the end, so we need to adjust it to the new
+      // end of the array.
+      info.Offset = lastIndex - originalOffset - 1;
+    }
+  }
+
   VTKM_CONT static void Fill(const std::vector<vtkm::cont::internal::Buffer>&,
                              const T&,
                              vtkm::Id,

vtkm/cont/testing/UnitTestArrayExtractComponent.cxx

@@ -25,6 +25,10 @@
 
 #include <vtkm/cont/testing/Testing.h>
 
+#include <algorithm>
+#include <random>
+#include <vector>
+
 namespace
 {
 
@@ -73,22 +77,38 @@ void CheckOutputArray(const vtkm::cont::ArrayHandle<T, S>& originalArray)
   //vtkm::cont::printSummary_ArrayHandle(originalArray, std::cout);
 
   vtkm::cont::ArrayHandle<T, S> outputArray;
-  outputArray.Allocate(originalArray.GetNumberOfValues());
 
   using FlatVec = vtkm::VecFlat<T>;
   using ComponentType = typename FlatVec::ComponentType;
   constexpr vtkm::IdComponent numComponents = FlatVec::NUM_COMPONENTS;
+
+  // Extract all the stride arrays first, and then allocate them later. This tests to
+  // to make sure that the independent allocation of all the extracted arrays are consistent
+  // and correct.
+  std::vector<std::pair<vtkm::cont::ArrayHandleStride<ComponentType>,
+                        vtkm::cont::ArrayHandleStride<ComponentType>>>
+    componentArrays;
+  componentArrays.reserve(static_cast<std::size_t>(numComponents));
   for (vtkm::IdComponent componentId = 0; componentId < numComponents; ++componentId)
   {
-    vtkm::cont::ArrayHandleStride<ComponentType> inComponentArray =
-      vtkm::cont::ArrayExtractComponent(originalArray, numComponents - componentId - 1);
-    vtkm::cont::ArrayHandleStride<ComponentType> outComponentArray =
-      vtkm::cont::ArrayExtractComponent(outputArray, componentId, vtkm::CopyFlag::Off);
+    componentArrays.emplace_back(
+      vtkm::cont::ArrayExtractComponent(originalArray, numComponents - componentId - 1),
+      vtkm::cont::ArrayExtractComponent(outputArray, componentId, vtkm::CopyFlag::Off));
+  }
 
-    auto inPortal = inComponentArray.ReadPortal();
-    auto outPortal = outComponentArray.WritePortal();
-    VTKM_TEST_ASSERT(inComponentArray.GetNumberOfValues() == originalArray.GetNumberOfValues());
-    VTKM_TEST_ASSERT(outComponentArray.GetNumberOfValues() == originalArray.GetNumberOfValues());
+  // Shuffle the component arrays to ensure the allocation/copy can occur in any order.
+  std::random_device rd;
+  std::default_random_engine rng(rd());
+  std::shuffle(componentArrays.begin(), componentArrays.end(), rng);
+
+  for (auto& inOutArrays : componentArrays)
+  {
+    inOutArrays.second.Allocate(originalArray.GetNumberOfValues());
+
+    auto inPortal = inOutArrays.first.ReadPortal();
+    auto outPortal = inOutArrays.second.WritePortal();
+    VTKM_TEST_ASSERT(inPortal.GetNumberOfValues() == originalArray.GetNumberOfValues());
+    VTKM_TEST_ASSERT(outPortal.GetNumberOfValues() == originalArray.GetNumberOfValues());
     for (vtkm::Id arrayIndex = 0; arrayIndex < originalArray.GetNumberOfValues(); ++arrayIndex)
     {
       outPortal.Set(arrayIndex, inPortal.Get(arrayIndex));
@@ -173,7 +193,12 @@ void DoTest()
     auto compositeArray = vtkm::cont::make_ArrayHandleCompositeVector(array0, array1);
     CheckOutputArray(compositeArray);
 
-    CheckOutputArray(vtkm::cont::make_ArrayHandleExtractComponent(compositeArray, 1));
+    // Note that when the extracted component array gets allocated, it only allocates the
+    // array it was given. This is a weird case when using `ArrayHandleExtractComponent`
+    // on something that has multiple arrays as input. It works fine if all components get
+    // extracted and updated, but can cause issues if only one is resized. In this case
+    // just test the input.
+    CheckInputArray(vtkm::cont::make_ArrayHandleExtractComponent(compositeArray, 1));
   }
 
   {

vtkm/cont/testing/UnitTestArrayHandleRecombineVec.cxx

@@ -69,7 +69,6 @@ struct TestRecombineVecAsOutput
     SetPortal(baseArray.WritePortal());
 
     vtkm::cont::ArrayHandle<T> outputArray;
-    outputArray.Allocate(ARRAY_SIZE); // Cannot resize after recombine
 
     using VTraits = vtkm::VecTraits<T>;
     vtkm::cont::ArrayHandleRecombineVec<typename VTraits::ComponentType> recombinedArray;
@@ -78,7 +77,6 @@ struct TestRecombineVecAsOutput
       recombinedArray.AppendComponentArray(vtkm::cont::ArrayExtractComponent(outputArray, cIndex));
     }
     VTKM_TEST_ASSERT(recombinedArray.GetNumberOfComponents() == VTraits::NUM_COMPONENTS);
-    VTKM_TEST_ASSERT(recombinedArray.GetNumberOfValues() == ARRAY_SIZE);
 
     vtkm::cont::Invoker invoke;
     invoke(PassThrough{}, baseArray, recombinedArray);

vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx

@@ -350,6 +350,43 @@ void TryAsMultiplexer(vtkm::cont::UnknownArrayHandle sourceArray)
 #endif
 }
 
+struct SimpleRecombineCopy
+{
+  template <typename T>
+  void operator()(const vtkm::cont::ArrayHandleRecombineVec<T>& inputArray,
+                  const vtkm::cont::UnknownArrayHandle& output) const
+  {
+    vtkm::cont::ArrayHandleRecombineVec<T> outputArray =
+      output.ExtractArrayFromComponents<T>(vtkm::CopyFlag::Off);
+    vtkm::Id size = inputArray.GetNumberOfValues();
+    outputArray.Allocate(size);
+    auto inputPortal = inputArray.ReadPortal();
+    auto outputPortal = outputArray.WritePortal();
+
+    for (vtkm::Id index = 0; index < size; ++index)
+    {
+      outputPortal.Set(index, inputPortal.Get(index));
+    }
+  }
+};
+
+template <typename T>
+void TryExtractArray(const vtkm::cont::UnknownArrayHandle& originalArray)
+{
+  // This check should already have been performed by caller, but just in case.
+  CheckUnknownArray<vtkm::List<T>, VTKM_DEFAULT_STORAGE_LIST>(originalArray,
+                                                              vtkm::VecTraits<T>::NUM_COMPONENTS);
+
+  std::cout << "Create new instance of array." << std::endl;
+  vtkm::cont::UnknownArrayHandle newArray = originalArray.NewInstanceBasic();
+
+  std::cout << "Do CastAndCallWithExtractedArray." << std::endl;
+  originalArray.CastAndCallWithExtractedArray(SimpleRecombineCopy{}, newArray);
+
+  CheckUnknownArray<vtkm::List<T>, VTKM_DEFAULT_STORAGE_LIST>(newArray,
+                                                              vtkm::VecTraits<T>::NUM_COMPONENTS);
+}
+
 template <typename T>
 void TryDefaultType()
 {
@@ -360,6 +397,8 @@ void TryDefaultType()
   TryNewInstance<T>(array);
 
   TryAsMultiplexer<T>(array);
+
+  TryExtractArray<T>(array);
 }
 
 struct TryBasicVTKmType