Skip to content

Commit f8f4090

Browse files
committed
expressions: address PR review (Phase E)
- GeneralRepermuteOp: store/apply only the OUTER result-layout permutation. The streaming re-permute wrapper exists to reorder a general product's outer (result) layout; inner (within-cell) permutation of ToT results is handled separately (init_struct_general / implicit_permute_inner_). Using the full bipartite perm_ would also permute inner cells -- a no-op when the inner perm is identity, but a latent double-apply if an inner perm is ever deferred to a downstream op. Pass outer(perm_) into the op; the host UnaryEvalImpl still receives full perm_ for ordinal/trange remap. - MultEngine down-pass: materialize each child demand as a named lvalue before preferred_layout(), which returns a reference to its argument for leaf/binary engines -- avoids a needlessly fragile bind-to-temporary.
1 parent 7c5d1c2 commit f8f4090

2 files changed

Lines changed: 19 additions & 7 deletions

File tree

src/TiledArray/expressions/cont_engine.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -851,7 +851,12 @@ class ContEngine : public BinaryEngine<Derived> {
851851
typedef value_type result_type;
852852
typedef value_type argument_type;
853853
static constexpr bool is_consumable = false;
854-
BipartitePermutation perm;
854+
/// Only the *outer* (result-layout) permutation is applied here; inner
855+
/// (within-cell) permutation of tensor-of-tensor results is handled
856+
/// separately (see init_struct_general / implicit_permute_inner_), so this
857+
/// op stores a plain outer Permutation to avoid accidentally permuting
858+
/// inner contents.
859+
Permutation perm;
855860
/// false when the consumer fuses the permutation into its own operation
856861
/// (implicit permute, e.g. a transposed GEMM): then only the tile
857862
/// ordinals/trange are remapped (by the host UnaryEvalImpl) and the tile
@@ -917,7 +922,7 @@ class ContEngine : public BinaryEngine<Derived> {
917922
std::shared_ptr<repermute_impl_type> wrapper =
918923
std::make_shared<repermute_impl_type>(
919924
canonical, *world_, trange_, shape_, pmap_, perm_,
920-
GeneralRepermuteOp{perm_, !this->implicit_permute_outer_});
925+
GeneralRepermuteOp{outer(perm_), !this->implicit_permute_outer_});
921926
return dist_eval_type(wrapper);
922927
}
923928

src/TiledArray/expressions/mult_engine.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -328,11 +328,18 @@ class MultEngine : public ContEngine<MultEngine<Left, Right, Result>> {
328328
// each child dictates the ORDER of its demand (preferred_layout): a
329329
// product child reorders to its canonical (h, eA, eB) layout -- a
330330
// general product cannot host a result permutation, and contraction
331-
// consumers absorb any child layout via the GEMM transpose forms
332-
BinaryEngine_::left_.init_indices(BinaryEngine_::left_.preferred_layout(
333-
bipartite_demand(avail_l, avail_r, target_indices)));
334-
BinaryEngine_::right_.init_indices(BinaryEngine_::right_.preferred_layout(
335-
bipartite_demand(avail_r, avail_l, target_indices)));
331+
// consumers absorb any child layout via the GEMM transpose forms.
332+
// Materialize the demands as named lvalues: some preferred_layout()
333+
// overloads (leaf/binary/unary) return a reference to their argument, so
334+
// binding that to a temporary demand would be needlessly fragile.
335+
const BipartiteIndexList left_demand =
336+
bipartite_demand(avail_l, avail_r, target_indices);
337+
const BipartiteIndexList right_demand =
338+
bipartite_demand(avail_r, avail_l, target_indices);
339+
BinaryEngine_::left_.init_indices(
340+
BinaryEngine_::left_.preferred_layout(left_demand));
341+
BinaryEngine_::right_.init_indices(
342+
BinaryEngine_::right_.preferred_layout(right_demand));
336343
}
337344

338345
this->product_type_ = compute_product_type(

0 commit comments

Comments
 (0)