Skip to content

Commit 711116f

Browse files
committed
summa: fix h-grouped result-tile owner; enable the 3-d grid by default
The deadlock that gated the h-grouped (hgroups>1) path: Summa's output get_tile(i) computed the result tile's source rank from the GROUP-LOCAL process-grid coordinates (proc_row*proc_cols + proc_col) but never added the world-rank offset of the tile's slab group. So a non-zero group recv'd each result tile from the wrong (group-0) rank, which never sent it -> hang. set_tile already routed correctly via the h-grouped pmap; only get_tile disagreed. Fix: result_tile_owner(i) = within-group cyclic owner + (slab_group * group_size), matching the h-grouped SlabbedPmap's owner() (and thus set_tile's destination). Store group_size_ for the offset. hgroups==1 is unchanged (offset 0). With this the heuristic is enabled by default (spread surplus ranks over the slab dimension when M*N tiles < P), so no-external products distribute their result across slab groups instead of piling on one rank. Validated np=2: all general_product no-external cases (dense/ToT/sparse) + the distributed suite (now incl. dist_no_externals_hgrouped, which asserts the result spans >1 rank). np=1 + full regression unchanged.
1 parent da4c250 commit 711116f

3 files changed

Lines changed: 59 additions & 26 deletions

File tree

src/TiledArray/dist_eval/contraction_eval.h

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,33 @@ class Summa
134134
// constructed over the group's rank interval). hgroups_ == 1 is the
135135
// ordinary shared-grid batched contraction.
136136
const ordinal_type hgroups_; ///< Number of slab (h) groups
137+
const ordinal_type group_size_; ///< World ranks per slab group (the
138+
///< group of slab h spans world ranks
139+
///< [(h % hgroups_) * group_size_, ...))
137140
const ordinal_type first_slab_; ///< This rank's group's first slab (== its
138141
///< group index), or nh_ if this rank is
139142
///< in no group (idle for this eval)
140143
const ordinal_type my_slabs_; ///< Number of slabs of this rank's group
141144

145+
/// \return the world rank that owns result tile \p i: the within-group
146+
/// owner (from the group-local process grid) shifted by the world-rank
147+
/// offset of the group that owns \p i's slab. For hgroups_ == 1 the offset
148+
/// is 0 and this is the ordinary cyclic owner.
149+
ProcessID result_tile_owner(const ordinal_type i) const {
150+
const ordinal_type source_index = DistEvalImpl_::perm_index_to_source(i);
151+
// owner is independent of slab index *within a group*
152+
const ordinal_type slab_index = source_index % result_slab_size_;
153+
const ordinal_type tile_row = slab_index / proc_grid_.cols();
154+
const ordinal_type tile_col = slab_index % proc_grid_.cols();
155+
const ordinal_type proc_row = tile_row % proc_grid_.proc_rows();
156+
const ordinal_type proc_col = tile_col % proc_grid_.proc_cols();
157+
const ProcessID within_group = proc_row * proc_grid_.proc_cols() + proc_col;
158+
// shift by the offset of the group that owns this tile's slab
159+
const ordinal_type slab = source_index / result_slab_size_;
160+
const ordinal_type group = (hgroups_ > 1ul) ? (slab % hgroups_) : 0ul;
161+
return ProcessID(group * group_size_) + within_group;
162+
}
163+
142164
/// \return the slab index of SUMMA step \p s
143165
ordinal_type step_h(const ordinal_type s) const { return s / k_; }
144166
/// \return the within-slab inner-dimension index of SUMMA step \p s
@@ -1728,6 +1750,7 @@ class Summa
17281750
right_slab_size_(right.size() / nh),
17291751
result_slab_size_(proc_grid.rows() * proc_grid.cols()),
17301752
hgroups_(hgroups),
1753+
group_size_(group_size),
17311754
first_slab_(compute_first_slab(world, nh, hgroups, group_size)),
17321755
my_slabs_(first_slab_ < nh
17331756
? (nh - first_slab_ + hgroups - 1ul) / hgroups
@@ -1777,18 +1800,11 @@ class Summa
17771800
TA_ASSERT(TensorImpl_::is_local(i));
17781801
TA_ASSERT(!TensorImpl_::is_zero(i));
17791802

1780-
const ordinal_type source_index = DistEvalImpl_::perm_index_to_source(i);
1781-
1782-
// Compute tile coordinate in tile grid (the owner of a tile is
1783-
// independent of its slab index)
1784-
const ordinal_type slab_index = source_index % result_slab_size_;
1785-
const ordinal_type tile_row = slab_index / proc_grid_.cols();
1786-
const ordinal_type tile_col = slab_index % proc_grid_.cols();
1787-
// Compute process coordinate of tile in the process grid
1788-
const ordinal_type proc_row = tile_row % proc_grid_.proc_rows();
1789-
const ordinal_type proc_col = tile_col % proc_grid_.proc_cols();
1790-
// Compute the process that owns tile
1791-
const ProcessID source = proc_row * proc_grid_.proc_cols() + proc_col;
1803+
// The process that owns tile i: the within-group cyclic owner shifted by
1804+
// the world-rank offset of the tile's slab group (see
1805+
// result_tile_owner). For hgroups_ == 1 this is the ordinary cyclic
1806+
// owner over the whole world.
1807+
const ProcessID source = result_tile_owner(i);
17921808

17931809
const madness::DistributedID key(DistEvalImpl_::id(), i);
17941810
return TensorImpl_::world().gop.template recv<value_type>(source, key);

src/TiledArray/expressions/cont_engine.h

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -859,29 +859,19 @@ class ContEngine : public BinaryEngine<Derived> {
859859
// h % hgroups_ of hgroup_size_ contiguous ranks; the division
860860
// remainder ranks idle for this evaluation). For a no-external
861861
// product (M == N == 1) this degenerates to an effectively 1-d grid
862-
// over the slabs.
863-
//
864-
// N.B. EXPERIMENTAL / WIP: the h-grouped (hgroups_ > 1) path is not
865-
// yet correct at np > 1 -- a group other than group 0 sets its result
866-
// tiles but the reduce-task result future never resolves (operands are
867-
// ready, the reduce receives its adds + submit, yet completion never
868-
// fires; see docs/superpowers/plans/2026-06-12-ta-summa-3d-grid.md for
869-
// the precise deadlock state). Until that is resolved hgroups_ defaults
870-
// to 1 (no behavior change: no-external products evaluate via the
871-
// existing degenerate single-grid path, correct but not h-parallel).
872-
// Set TA_SUMMA_HGROUPS=<n> to opt into the WIP path for debugging.
862+
// over the slabs. TA_SUMMA_HGROUPS overrides the heuristic.
873863
const size_type P = world->size();
874864
hgroups_ = 1ul;
875865
if (n_slabs_ > 1ul && P > 1ul) {
866+
const size_type p2d_cap = std::min<size_type>(P, M * N);
867+
hgroups_ = std::min<size_type>(n_slabs_,
868+
std::max<size_type>(1ul, P / p2d_cap));
876869
if (const char* hg = std::getenv("TA_SUMMA_HGROUPS")) {
877870
const long requested = std::atol(hg);
878871
if (requested >= 1)
879872
hgroups_ = std::min<size_type>(
880873
n_slabs_, std::min<size_type>(P, size_type(requested)));
881874
}
882-
// default heuristic (currently disabled pending the WIP fix):
883-
// const size_type p2d_cap = std::min<size_type>(P, M * N);
884-
// hgroups_ = std::min(n_slabs_, std::max(1ul, P / p2d_cap));
885875
}
886876
hgroup_size_ = P / hgroups_;
887877

tests/general_product.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include "tiledarray.h"
99
#include "unit_test_config.h"
1010

11+
#include <set>
12+
1113
BOOST_AUTO_TEST_SUITE(general_product_suite, TA_UT_LABEL_SERIAL)
1214

1315
namespace TA = TiledArray;
@@ -1455,4 +1457,29 @@ BOOST_AUTO_TEST_CASE(dist_inner_node_thc) {
14551457
BOOST_CHECK_SMALL(diff_norm(g, g_ref, "p,q,r,s"), 1e-10);
14561458
}
14571459

1460+
BOOST_AUTO_TEST_CASE(dist_no_externals_hgrouped) {
1461+
// a no-external product with several slabs: at np>1 the heuristic engages
1462+
// the 3-d (h-grouped) grid (M=N=1 ⇒ each slab gets its own rank group),
1463+
// so the result tiles are distributed across the slab groups rather than
1464+
// piled on one rank. Exercises the cross-group result-tile transfer.
1465+
auto& world = TA::get_default_world();
1466+
ForceLegacyEinsum legacy_oracle;
1467+
TA::TiledRange tr{{0, 2, 4, 6}, {0, 3, 5}}; // i (3 tiles), j
1468+
auto a = make_patterned_array(world, tr, 1.0);
1469+
auto b = make_patterned_array(world, tr, 2.0);
1470+
TA::TArrayD c;
1471+
BOOST_REQUIRE_NO_THROW(c("i") = a("i,j") * b("i,j"));
1472+
auto c_ref = TA::einsum(a("i,j"), b("i,j"), "i");
1473+
BOOST_CHECK_SMALL(diff_norm(c, c_ref, "i"), 1e-10);
1474+
1475+
// at np>1 the no-external result must NOT all live on one rank (the
1476+
// degeneracy the 3-d grid fixes)
1477+
if (world.size() > 1) {
1478+
std::set<std::size_t> owners;
1479+
for (std::size_t o = 0; o < c.trange().tiles_range().volume(); ++o)
1480+
owners.insert(c.pmap()->owner(o));
1481+
BOOST_CHECK_GT(owners.size(), 1u);
1482+
}
1483+
}
1484+
14581485
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)