diff --git a/docs/changelog/deprecate-reduce-by-key-count.md b/docs/changelog/deprecate-reduce-by-key-count.md new file mode 100644 index 000000000..c5f15a84f --- /dev/null +++ b/docs/changelog/deprecate-reduce-by-key-count.md @@ -0,0 +1,12 @@ +# Deprecate the GetCounts() method in Keys objects + +The `vtkm::worklet::Keys` object held a `SortedValuesMap` array, an +`Offsets` array, a `Counts` array, and (optionally) a `UniqueKeys` array. +Of these, the `Counts` array is redundant because the counts are trivially +computed by subtracting adjacent entries in the offsets array. This pattern +shows up a lot in VTK-m, and most places we have moved to removing the +counts and just using the offsets. + +This change removes the `Count` array from the `Keys` object. Where the +count is needed internally, adjacent offsets are subtracted. The deprecated +`GetCounts` method is implemented by copying values into a new array. diff --git a/vtkm/exec/arg/ThreadIndicesReduceByKey.h b/vtkm/exec/arg/ThreadIndicesReduceByKey.h index cedee477e..2742169e2 100644 --- a/vtkm/exec/arg/ThreadIndicesReduceByKey.h +++ b/vtkm/exec/arg/ThreadIndicesReduceByKey.h @@ -42,7 +42,8 @@ public: const vtkm::exec::internal::ReduceByKeyLookupBase& keyLookup) : Superclass(threadIndex, inIndex, visitIndex, outIndex) , ValueOffset(keyLookup.Offsets.Get(inIndex)) - , NumberOfValues(keyLookup.Counts.Get(inIndex)) + , NumberOfValues(static_cast(keyLookup.Offsets.Get(inIndex + 1) - + keyLookup.Offsets.Get(inIndex))) { } diff --git a/vtkm/exec/internal/ReduceByKeyLookup.h b/vtkm/exec/internal/ReduceByKeyLookup.h index 7111b71fb..0f8e5aef2 100644 --- a/vtkm/exec/internal/ReduceByKeyLookup.h +++ b/vtkm/exec/internal/ReduceByKeyLookup.h @@ -35,15 +35,11 @@ struct ReduceByKeyLookupBase IdPortalType SortedValuesMap; IdPortalType Offsets; - IdComponentPortalType Counts; VTKM_EXEC_CONT - ReduceByKeyLookupBase(const IdPortalType& sortedValuesMap, - const IdPortalType& offsets, - const IdComponentPortalType& counts) + ReduceByKeyLookupBase(const IdPortalType& sortedValuesMap, const IdPortalType& offsets) : SortedValuesMap(sortedValuesMap) , Offsets(offsets) - , Counts(counts) { } @@ -68,9 +64,8 @@ struct ReduceByKeyLookup : ReduceByKeyLookupBase(sortedValuesMap, offsets, counts) + const IdPortalType& offsets) + : ReduceByKeyLookupBase(sortedValuesMap, offsets) , UniqueKeys(uniqueKeys) { } diff --git a/vtkm/worklet/Keys.h b/vtkm/worklet/Keys.h index e19562065..f0be65925 100644 --- a/vtkm/worklet/Keys.h +++ b/vtkm/worklet/Keys.h @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -52,7 +53,7 @@ public: ~KeysBase() = default; VTKM_CONT - vtkm::Id GetInputRange() const { return this->Counts.GetNumberOfValues(); } + vtkm::Id GetInputRange() const { return this->Offsets.GetNumberOfValues() - 1; } VTKM_CONT vtkm::cont::ArrayHandle GetSortedValuesMap() const { return this->SortedValuesMap; } @@ -60,8 +61,9 @@ public: VTKM_CONT vtkm::cont::ArrayHandle GetOffsets() const { return this->Offsets; } + VTKM_DEPRECATED(2.2, "Use the `GetOffsets()` array in an `ArrayHandleOffsetsToNumComponents`.") VTKM_CONT - vtkm::cont::ArrayHandle GetCounts() const { return this->Counts; } + vtkm::cont::ArrayHandle GetCounts() const; VTKM_CONT vtkm::Id GetNumberOfValues() const { return this->SortedValuesMap.GetNumberOfValues(); } @@ -74,15 +76,14 @@ public: vtkm::cont::Token& token) const { return ExecLookup(this->SortedValuesMap.PrepareForInput(device, token), - this->Offsets.PrepareForInput(device, token), - this->Counts.PrepareForInput(device, token)); + this->Offsets.PrepareForInput(device, token)); } VTKM_CONT bool operator==(const vtkm::worklet::internal::KeysBase& other) const { return ((this->SortedValuesMap == other.SortedValuesMap) && (this->Offsets == other.Offsets) && - (this->Counts == other.Counts)); + (this->Offsets == other.Offsets)); } VTKM_CONT @@ -96,7 +97,6 @@ protected: vtkm::cont::ArrayHandle SortedValuesMap; vtkm::cont::ArrayHandle Offsets; - vtkm::cont::ArrayHandle Counts; }; } // namespace internal @@ -184,16 +184,14 @@ public: { return ExecLookup(this->UniqueKeys.PrepareForInput(device, token), this->SortedValuesMap.PrepareForInput(device, token), - this->Offsets.PrepareForInput(device, token), - this->Counts.PrepareForInput(device, token)); + this->Offsets.PrepareForInput(device, token)); } VTKM_CONT bool operator==(const vtkm::worklet::Keys& other) const { return ((this->UniqueKeys == other.UniqueKeys) && - (this->SortedValuesMap == other.SortedValuesMap) && (this->Offsets == other.Offsets) && - (this->Counts == other.Counts)); + (this->SortedValuesMap == other.SortedValuesMap) && (this->Offsets == other.Offsets)); } VTKM_CONT diff --git a/vtkm/worklet/Keys.hxx b/vtkm/worklet/Keys.hxx index 0828bf0d3..4d77024c3 100644 --- a/vtkm/worklet/Keys.hxx +++ b/vtkm/worklet/Keys.hxx @@ -87,16 +87,17 @@ VTKM_CONT void Keys::BuildArraysInternal(KeyArrayType& keys, vtkm::cont::Devi vtkm::cont::Algorithm::SortByKey(device, keys, this->SortedValuesMap); // Find the unique keys and the number of values per key. + vtkm::cont::ArrayHandle counts; vtkm::cont::Algorithm::ReduceByKey(device, keys, vtkm::cont::ArrayHandleConstant(1, numKeys), this->UniqueKeys, - this->Counts, + counts, vtkm::Sum()); // Get the offsets from the counts with a scan. vtkm::cont::Algorithm::ScanExtended( - device, vtkm::cont::make_ArrayHandleCast(this->Counts, vtkm::Id()), this->Offsets); + device, vtkm::cont::make_ArrayHandleCast(counts, vtkm::Id()), this->Offsets); VTKM_ASSERT(numKeys == vtkm::cont::ArrayGetValue(this->Offsets.GetNumberOfValues() - 1, this->Offsets)); @@ -116,16 +117,17 @@ VTKM_CONT void Keys::BuildArraysInternalStable(const KeyArrayType& keys, auto sortedKeys = vtkm::cont::make_ArrayHandlePermutation(this->SortedValuesMap, keys); // Find the unique keys and the number of values per key. + vtkm::cont::ArrayHandle counts; vtkm::cont::Algorithm::ReduceByKey(device, sortedKeys, vtkm::cont::ArrayHandleConstant(1, numKeys), this->UniqueKeys, - this->Counts, + counts, vtkm::Sum()); // Get the offsets from the counts with a scan. vtkm::cont::Algorithm::ScanExtended( - device, vtkm::cont::make_ArrayHandleCast(this->Counts, vtkm::Id()), this->Offsets); + device, vtkm::cont::make_ArrayHandleCast(counts, vtkm::Id()), this->Offsets); VTKM_ASSERT(numKeys == vtkm::cont::ArrayGetValue(this->Offsets.GetNumberOfValues() - 1, this->Offsets)); diff --git a/vtkm/worklet/KeysSignedTypes.cxx b/vtkm/worklet/KeysSignedTypes.cxx index f3b170edc..d047d0f56 100644 --- a/vtkm/worklet/KeysSignedTypes.cxx +++ b/vtkm/worklet/KeysSignedTypes.cxx @@ -27,3 +27,15 @@ VTK_M_KEYS_EXPORT(vtkm::IdComponent); #endif #undef VTK_M_KEYS_EXPORT + +// Putting this deprecated implementation here because I am too lazy to create +// a separate source file just for a deprecated method. +#include +#include +vtkm::cont::ArrayHandle vtkm::worklet::internal::KeysBase::GetCounts() const +{ + vtkm::cont::ArrayHandle counts; + vtkm::cont::ArrayCopyDevice( + vtkm::cont::make_ArrayHandleOffsetsToNumComponents(this->GetOffsets()), counts); + return counts; +} diff --git a/vtkm/worklet/testing/UnitTestKeys.cxx b/vtkm/worklet/testing/UnitTestKeys.cxx index 668a130f5..bfd058067 100644 --- a/vtkm/worklet/testing/UnitTestKeys.cxx +++ b/vtkm/worklet/testing/UnitTestKeys.cxx @@ -20,25 +20,24 @@ namespace static constexpr vtkm::Id ARRAY_SIZE = 1033; static constexpr vtkm::Id NUM_UNIQUE = ARRAY_SIZE / 10; -template +template void CheckKeyReduce(const KeyPortal& originalKeys, const KeyPortal& uniqueKeys, const IdPortal& sortedValuesMap, - const IdPortal& offsets, - const IdComponentPortal& counts) + const IdPortal& offsets) { using KeyType = typename KeyPortal::ValueType; vtkm::Id originalSize = originalKeys.GetNumberOfValues(); vtkm::Id uniqueSize = uniqueKeys.GetNumberOfValues(); VTKM_TEST_ASSERT(originalSize == sortedValuesMap.GetNumberOfValues(), "Inconsistent array size."); VTKM_TEST_ASSERT(uniqueSize == offsets.GetNumberOfValues() - 1, "Inconsistent array size."); - VTKM_TEST_ASSERT(uniqueSize == counts.GetNumberOfValues(), "Inconsistent array size."); for (vtkm::Id uniqueIndex = 0; uniqueIndex < uniqueSize; uniqueIndex++) { KeyType key = uniqueKeys.Get(uniqueIndex); vtkm::Id offset = offsets.Get(uniqueIndex); - vtkm::IdComponent groupCount = counts.Get(uniqueIndex); + vtkm::IdComponent groupCount = + static_cast(offsets.Get(uniqueIndex + 1) - offset); for (vtkm::IdComponent groupIndex = 0; groupIndex < groupCount; groupIndex++) { vtkm::Id originalIndex = sortedValuesMap.Get(offset + groupIndex); @@ -69,8 +68,7 @@ void TryKeyType(KeyType) CheckKeyReduce(keyArray.ReadPortal(), keys.GetUniqueKeys().ReadPortal(), keys.GetSortedValuesMap().ReadPortal(), - keys.GetOffsets().ReadPortal(), - keys.GetCounts().ReadPortal()); + keys.GetOffsets().ReadPortal()); } void TestKeys() diff --git a/vtkm/worklet/testing/UnitTestWorkletReduceByKey.cxx b/vtkm/worklet/testing/UnitTestWorkletReduceByKey.cxx index 43c7d00a5..0232edc0e 100644 --- a/vtkm/worklet/testing/UnitTestWorkletReduceByKey.cxx +++ b/vtkm/worklet/testing/UnitTestWorkletReduceByKey.cxx @@ -131,7 +131,6 @@ void TryKeyType(KeyType) vtkm::worklet::Keys keys(sortedKeys); vtkm::cont::printSummary_ArrayHandle(keys.GetUniqueKeys(), std::cout); vtkm::cont::printSummary_ArrayHandle(keys.GetOffsets(), std::cout); - vtkm::cont::printSummary_ArrayHandle(keys.GetCounts(), std::cout); vtkm::cont::ArrayHandle valuesToModify; valuesToModify.Allocate(ARRAY_SIZE);