Skip to content

Commit bf43a86

Browse files
committed
summa: address review comments on the 3-d batched grid
- contraction_eval: clamp the SUMMA step-task pipeline depth to my_steps() (this rank's group's step count) instead of nsteps_. In the 3-d (proc_h_ > 1) case my_steps() < nsteps_, so clamping to nsteps_ pre-spawned surplus step tasks that all resolved to the terminating step (k_ == nsteps_). No-op for the 2-d path (my_slabs_ == nh_). - cont_engine: keep proc_h_stride_ == 0 for the ungrouped 2-d case (proc_h_ == 1), matching the field's documented invariant; only the grouped (proc_h_ > 1) grid uses P / proc_h_. - general_product test: correct the distributed suite header comment -- dist_inner_node_thc validates against explicit binary intermediates, not the legacy einsum oracle.
1 parent 7b4319c commit bf43a86

3 files changed

Lines changed: 16 additions & 5 deletions

File tree

src/TiledArray/dist_eval/contraction_eval.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,8 +1515,12 @@ class Summa
15151515

15161516
template <typename Derived>
15171517
void make_next_step_tasks(Derived* task, ordinal_type depth) {
1518-
// Set the depth to be no greater than the maximum number steps
1519-
if (depth > owner_->nsteps_) depth = owner_->nsteps_;
1518+
// Set the depth to be no greater than the number of SUMMA steps this
1519+
// rank's group actually executes. In the 2-d (proc_h_ == 1) case this is
1520+
// nsteps_ (my_slabs_ == nh_); in the 3-d (proc_h_ > 1) case my_steps() <
1521+
// nsteps_, and clamping to nsteps_ would pre-spawn surplus step tasks
1522+
// that all resolve to the terminating step (k_ == nsteps_).
1523+
if (depth > owner_->my_steps()) depth = owner_->my_steps();
15201524

15211525
// Spawn n=depth step tasks
15221526
for (; depth > 0ul; --depth) {

src/TiledArray/expressions/cont_engine.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,10 @@ class ContEngine : public BinaryEngine<Derived> {
874874
proc_h_ = std::min<size_type>(n_slabs_,
875875
std::max<size_type>(1ul, P / p2d_cap));
876876
}
877-
proc_h_stride_ = P / proc_h_;
877+
// keep the invariant proc_h_ == 1 => proc_h_stride_ == 0 (the ungrouped
878+
// 2-d case) so downstream logic can key off either field; for grouped
879+
// grids it is the per-plane world-rank count P / proc_h_.
880+
proc_h_stride_ = (proc_h_ == 1ul) ? 0ul : P / proc_h_;
878881

879882
if (proc_h_ == 1ul) {
880883
// Construct the per-slab process grid over the whole world.

tests/general_product.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,8 +1350,12 @@ BOOST_AUTO_TEST_SUITE_END()
13501350
// + np=1 evaluation), this suite carries NO label, so the CI harness runs it
13511351
// at BOTH np=1 and np=2 (see tests/CMakeLists.txt: np-1 excludes @distributed,
13521352
// np-2 excludes @serial). It exercises the batched Summa across ranks -- a
1353-
// path the serial suite never covered. Each case differential-tests the
1354-
// expression route against the legacy sub-World einsum oracle.
1353+
// path the serial suite never covered. Most cases differential-test the
1354+
// expression route against the legacy sub-World einsum oracle; the exception
1355+
// is dist_inner_node_thc, which checks a single fused expression against a
1356+
// reference built from explicit binary intermediates (the same expression
1357+
// machinery), validating that nested general products at inner nodes agree
1358+
// with the step-by-step evaluation.
13551359
//
13561360
// Most cases evaluate via the ordinary 2-d (proc_h == 1) batched Summa;
13571361
// dist_no_externals_3d_grid exercises the 3-d (proc_h > 1) grid, where the

0 commit comments

Comments
 (0)