Skip to content

Commit 06b7523

Browse files
committed
Tensor::permute returns an owning tile for arena nested outers
For a Tensor<ArenaTensor<...>> outer tile, permute() rebuilt the outer cell array in permuted order but rebound the 8-byte cell views onto the source's slab (arena_permute_shallow), keeping the source alive through the new tile's deleter. The permuted tile therefore aliased the source. The expression layer consumes a permuted operand temporary in place (BinaryWrapper with a consumable left/right: `x(perm) - y` becomes subt_to on the permuted tile), so the subtraction wrote through into x: after `z("i,j,q,k;a") = x("q,k,i,j;a") - y("i,j,q,k;a")` the array x was left corrupted (its norm dropped from 2.5e5 to 1.2e2 in the reproducer, to 0 in another run). A permute-copy (`y(perm) = x(...)`) and owning Tensor<Tensor> inners were unaffected. permute() now copies every cell into a fresh single-page slab in permuted order (arena_permute_deep), the same ownership rule the generic Tensor(other, perm) constructor and owning_copy_of() already follow, so a permuted tile may be consumed without touching its source. The shallow helper stays for callers that only read the permuted view. Cost: one slab copy per outer permute of an arena tile. On a mixed nested-times-flat product whose result the engine permutes this is visible in isolation (0.13 s -> 0.19 s on the tests' benchmark product); on an MPQC CSV-CCSD water-8-mer iteration it is within noise (9.4 s -> 9.7 s DF+occupied batched, 3.3 s -> 3.4 s DF batched), with energies and residual norms unchanged to 12 digits. An rvalue permute for the engine's own result temporaries could restore the zero-copy path there. tests/arena_mixed_product_order.cpp: the second case (the reproducer) now passes; its note is updated.
1 parent 282a876 commit 06b7523

3 files changed

Lines changed: 62 additions & 11 deletions

File tree

‎src/TiledArray/tensor/arena_kernels.h‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,50 @@ OuterTensor arena_permute_shallow(const SrcOuterTensor& src, const Perm& perm) {
625625
return result;
626626
}
627627

628+
/// Deep outer permute of a slab-backed ToT outer tile: a fresh single-page
629+
/// slab holding a copy of every cell, laid out in permuted order. The owning
630+
/// counterpart of arena_permute_shallow(): the result shares nothing with
631+
/// `src`, so it may be consumed in place -- the expression layer consumes a
632+
/// permuted operand tile (`x(perm) - y`) -- without touching `src`; the
633+
/// shallow form aliased the source cells, and such a consume wrote through
634+
/// them. Inner cells keep their ranges (an inner permutation is a separate
635+
/// pass, arena_inner_permute()). Null cells stay null.
636+
template <typename OuterTensor, typename SrcOuterTensor, typename Perm>
637+
OuterTensor arena_permute_deep(const SrcOuterTensor& src, const Perm& perm) {
638+
using inner_t = typename OuterTensor::value_type;
639+
using elem_t = typename inner_t::value_type;
640+
using inner_range_t = typename inner_t::range_type;
641+
TA_ASSERT(perm);
642+
TA_ASSERT(perm.size() == src.range().rank());
643+
auto perm_range = perm * src.range();
644+
const std::size_t N_cells = src.range().volume();
645+
const std::size_t batch_sz = src.nbatch();
646+
// target cell ordinal -> source cell ordinal (within one batch)
647+
std::vector<std::size_t> src_of(N_cells);
648+
for (std::size_t s = 0; s < N_cells; ++s) {
649+
auto src_idx = src.range().idx(s);
650+
src_of[perm_range.ordinal(perm * src_idx)] = s;
651+
}
652+
auto range_fn = [&](std::size_t t_off) -> inner_range_t {
653+
const std::size_t b = t_off / N_cells, t = t_off % N_cells;
654+
const inner_t& c = src.data()[b * N_cells + src_of[t]];
655+
return c.empty() ? inner_range_t{} : c.range();
656+
};
657+
// every element of every live cell is written below; no zero-init
658+
OuterTensor result = arena_outer_init<OuterTensor>(
659+
perm_range, batch_sz, range_fn, alignof(elem_t), /*zero_init=*/false);
660+
for (std::size_t b = 0; b < batch_sz; ++b) {
661+
for (std::size_t t = 0; t < N_cells; ++t) {
662+
inner_t& dst = result.data()[b * N_cells + t];
663+
if (dst.empty()) continue;
664+
const inner_t& s = src.data()[b * N_cells + src_of[t]];
665+
TA_ASSERT(dst.size() == s.size());
666+
std::copy_n(s.data(), s.size(), dst.data());
667+
}
668+
}
669+
return result;
670+
}
671+
628672
/// Permute the inner modes of every cell of a slab-backed ToT outer tile.
629673
///
630674
/// Produces a fresh slab-backed tile with the same outer layout as `src`,

‎src/TiledArray/tensor/tensor.h‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,15 +1710,20 @@ class Tensor {
17101710
Tensor permute(const Perm& perm) const {
17111711
if constexpr (is_arena_tensor_v<value_type>) {
17121712
// View inner cells cannot be permuted in place; the owning tile
1713-
// rewrites its slab(s). The outer cells reorder shallowly (the 8-byte
1714-
// views are reindexed, the slab is shared via keep-alive); a
1715-
// non-trivial inner permutation rewrites every cell into a fresh slab.
1716-
// The generic Tensor(other, perm) ctor's allocate-then-fill shape does
1717-
// not fit the arena slab model, so route around it.
1713+
// rewrites its slab(s). The outer permutation copies every cell into a
1714+
// fresh slab in permuted order (arena_permute_deep): like the generic
1715+
// Tensor(other, perm) ctor, permute() returns an OWNING tile, which the
1716+
// expression layer may consume in place. (The earlier shallow form
1717+
// reindexed the 8-byte views over the shared source slab, and an
1718+
// in-place op on the permuted operand -- e.g. `x(perm) - y` -- wrote
1719+
// through into the source.) A non-trivial inner permutation rewrites
1720+
// every cell into a fresh slab as before. The generic ctor's
1721+
// allocate-then-fill shape does not fit the arena slab model, so route
1722+
// around it.
17181723
const auto outer_perm = outer(perm);
17191724
Tensor result =
17201725
(outer_perm && !outer_perm.is_identity())
1721-
? detail::arena_permute_shallow<Tensor>(*this, outer_perm)
1726+
? detail::arena_permute_deep<Tensor>(*this, outer_perm)
17221727
: *this;
17231728
if constexpr (detail::is_bipartite_permutation_v<Perm>) {
17241729
const auto inner_perm = inner(perm);

‎tests/arena_mixed_product_order.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@
2121
//
2222
// 2. permuted operand in a binary op: reading an arena ToT through a
2323
// PERMUTED annotation inside a binary expression (e.g. `x(perm) - y`)
24-
// destroyed the source: the permuted operand tile is a SHALLOW permute
25-
// (cells alias the source slab) and the in-place binary op wrote through
26-
// it. A permute-COPY (`y(perm) = x(...)`) was unaffected, and owning
27-
// (Tensor<Tensor>) inners were unaffected. The test asserts the source's
28-
// norm is unchanged after such a read.
24+
// destroyed the source: Tensor::permute() returned a SHALLOW permute
25+
// (cells aliasing the source slab) and the in-place binary op, which
26+
// consumes a permuted operand temporary, wrote through it. A
27+
// permute-COPY (`y(perm) = x(...)`) was unaffected, and owning
28+
// (Tensor<Tensor>) inners were unaffected. permute() now returns an
29+
// owning tile (arena_permute_deep); the test asserts the source's norm
30+
// is unchanged after such a read.
2931

3032
#include "TiledArray/tensor/arena_einsum.h"
3133
#include "TiledArray/tensor/arena_tensor.h"

0 commit comments

Comments
 (0)