Fix block-sparse sub-block assignment when RHS threshold is looser than dest

[evaleev]

Problem

a("i,j").block(lo,hi) = rhs("i,j") aborts with TA_ASSERT(!is_zero(i)) in ArrayImpl<..., SparsePolicy>::set (or hangs, under the throwing assert policy inside a task) when the RHS expression's shape carries a looser screening threshold than the destination array.

Expr::eval_to(BlkTsrExpr) builds the assigned sub-block's shape via SparseShape::update_block, which screens the updated region with the destination array's my_threshold_. The tile-write loop, however, wrote every tile the RHS (dist_eval) shape kept. A tile the RHS keeps but the merged result shape screens to zero was then written into a shape-zero slot, tripping the !is_zero assertion.

This surfaces downstream (e.g. a sub-block scatter that reconstructs a sparse result from independently-screened partials, where the partials were screened at a lower threshold than the pre-sized destination).

Fix

Make the destination shape authoritative in the block-write loop: only set tiles the merged result shape keeps. The RHS tile is still get()-consumed unconditionally — skipping the get() would strand the scheduled producer and deadlock dist_eval.wait().

DensePolicy is unaffected: DenseShape::is_zero is always false, so the added guard is inert and behavior is byte-identical.

Test

Adds expressions_sparse_block_assign_suite/block_assign_threshold_mismatch: a real TSpArrayD sub-block assignment whose source keeps a small-norm tile that the stricter destination threshold screens out. It asserts the assignment does not throw and that the destination threshold decides which tiles survive. The case fails (assertion) without the fix.

Verified locally: new case + expressions_sparse (52), dense expressions (52), and ToT expression suites all pass.

[Copilot]

Pull request overview

This PR fixes a correctness issue in sparse sub-block assignment (dest.block(...) = rhs) where the block-write loop could attempt to write tiles that the destination’s (stricter) sparse shape screens to zero, triggering ArrayImpl::set assertions (or hanging under throwing assert policy in tasks). The fix makes the destination block shape authoritative when deciding which RHS tiles to actually set, while still unconditionally consuming RHS tiles from dist_eval to avoid deadlocking dist_eval.wait().

Changes:

• Guard sub-block tile writes so only tiles kept by the merged destination result shape are written.
• Always dist_eval.get() consumed tiles even when they’ll be dropped, preventing evaluator deadlock.
• Add a regression test covering threshold-mismatch behavior for sparse block assignment.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/TiledArray/expressions/expr.h	Drops RHS tiles that the destination-updated block shape screens to zero, preventing set into shape-zero slots while still consuming RHS futures.
tests/expressions_sparse.cpp	Adds a regression test verifying block assignment behavior when RHS and destination screening thresholds differ.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[evaleev]

Good catch — fixed in 4e428a4. Replaced the manual capture + trailing restore with a local RAII ThresholdRestorer struct (ctor captures threshold(), dtor restores it), so a throw anywhere in the test body can no longer leak the modified global threshold into later cases.

[evaleev]

Revised in c4ffa72. Rather than a hand-rolled RAII guard, the test now routes every threshold change (including the restore) through a set_threshold helper that uses the documented, sanctioned idiom for changing the global static threshold: fence, then world.gop.serial_invoke([t]{ SparseShape<float>::threshold(t); }) — see the warning on SparseShape::threshold and the same pattern in conversions/truncate.h. This both keeps all ranks consistent under a fence (a bare setter, including my earlier RAII destructor, violated that) and removes the leaked-threshold concern.

[evaleev]

Correction to the above: the leading fence is unnecessary — world.gop.serial_invoke([t]{ SparseShape<float>::threshold(t); }) alone is the documented idiom (matches the example on SparseShape::threshold and conversions/truncate.h, neither of which fences first). Final form in 885f83c.