Skip to content

Commit 4765265

Browse files
authored
Merge pull request #538 from ValeevGroup/evaleev/fix/dist-array-fill-zero-is-sparse
DistArray::fill(0) produces an empty array if sparse
2 parents 5fbd2e6 + 02a473f commit 4765265

File tree

3 files changed

+25
-1
lines changed

3 files changed

+25
-1
lines changed

doc/dox/dev/Basic-Programming.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ for(TArrayD::iterator it = array.begin(); it != array.end(); ++it) {
533533
```
534534
The outer loop in this example iterates over the local tiles of `array`. Within the loop body we first create an tile (an object of `TArrayD::value_type` type, which in this case is `TA::Tensor<double>`). Then we loop over its elements and assign each to zero.
535535

536-
N.B. Of course, filling a DistArray with a constant is such a common use case that there's already a method for exactly that: `array.fill(0.0)`.
536+
N.B. Of course, filling a DistArray with a constant is such a common use case that there's already a method for exactly that: `array.fill(0.0)`. Note that for sparse arrays `fill(0.0)` will produce an empty array with no tiles, rather than an array full of tiles filled with zeroes.
537537

538538
There are more serious issues with the last example. First, it is too verbose. Second, it's not generic enough (i.e. trying to reuse it for a sparse DistArray would require changing a few lines). Both issues can be solved by using modern C++ features:
539539
```.cpp

src/TiledArray/dist_array.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,9 @@ class DistArray : public madness::archive::ParallelSerializableObject {
918918

919919
/// Fill all local tiles with the specified value
920920

921+
/// For sparse arrays, if \p value is zero (i.e., equal to
922+
/// `element_type()`), the array is empty (no nonzero tiles)
923+
/// and 0 is returned.
921924
/// \tparam fence If Fence::No, the operation will return early,
922925
/// before the tasks have completed
923926
/// \param[in] value What each local tile should be filled with.
@@ -930,6 +933,15 @@ class DistArray : public madness::archive::ParallelSerializableObject {
930933
template <Fence fence = Fence::No>
931934
std::int64_t fill(const element_type& value = numeric_type(),
932935
bool skip_set = false) {
936+
// for sparse arrays filled with zero, replace with an empty array
937+
if constexpr (!is_dense_v<Policy>) {
938+
if (value == element_type()) {
939+
*this = DistArray(
940+
world(), trange(),
941+
shape_type(typename shape_type::value_type(0), trange()), pmap());
942+
return 0;
943+
}
944+
}
933945
return fill_local<fence>(value, skip_set);
934946
}
935947

tests/dist_array.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,4 +1007,16 @@ BOOST_AUTO_TEST_CASE(size_of) {
10071007
BOOST_REQUIRE(sz0 == sz0_expected);
10081008
}
10091009

1010+
BOOST_FIXTURE_TEST_CASE(fill_zero_sparse, ArrayFixture) {
1011+
// construct a sparse array with some non-zero tiles and fill it
1012+
SpArrayN as(world, tr, TiledArray::SparseShape<float>(shape_tensor, tr));
1013+
as.fill(1);
1014+
BOOST_CHECK_GT(as.shape().nnz(), 0);
1015+
1016+
// fill with zero should result in an empty array (no tiles)
1017+
as.fill(0);
1018+
BOOST_CHECK_EQUAL(as.shape().nnz(), 0);
1019+
BOOST_CHECK(as.begin() == as.end());
1020+
}
1021+
10101022
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)