Generalize AsyCost

[ajay-mk]

Generalize AsyCost from O/V to arbitrary IndexSpace

AsyCost previously hard-coded asymptotic cost as a polynomial in exactly two orbital spaces: occupied (O) and virtual (V), with each entry storing a fixed (occ, virt) exponent pair.

This PR replaces the fixed O/V representation with a general map from IndexSpace to integer exponent, so a cost term is now a rational multiplier times a product of arbitrary space sizes raised to integer powers

Changes

• A cost entry now stores a map from IndexSpace to exponent instead of fixed occupied/virtual powers, so any number of spaces is supported.
• Constructors take an exponent map; ops() takes a map of space extents instead of two numbers.
• Printing uses base labels of each IndexSpace.
• Cost ordering now compares total scaling first, then breaks ties space by space.
• test_asy_cost.cpp and test_eval_node.cpp updated to construct costs via the exponent-map API, and new tests are added with general IndexSpaces.

[Copilot]

Pull request overview

This PR generalizes AsyCost from a fixed occupied/virtual (O/V) polynomial representation to a map keyed by arbitrary IndexSpace, enabling asymptotic cost and memory scalings to be expressed over any number of index spaces (including auxiliary and batching spaces).

Changes:

• Replaced O/V exponent pairs with an IndexSpace -> exponent map and updated printing/comparison semantics accordingly.
• Updated flop/memory accounting in eval_node to count indices by IndexSpace (including aux slots).
• Updated and expanded unit tests to construct/compare costs via the exponent-map API and to cover generalized spaces.

Reviewed changes

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

Show a summary per file

File	Description
SeQuant/core/asy_cost.hpp	Updates AsyCost public API and documentation to use IndexSpace-keyed exponent maps.
SeQuant/core/asy_cost.cpp	Implements generalized exponent-map storage, printing, ordering, and numeric evaluation via extents.
SeQuant/core/eval/eval_node.hpp	Updates flop/memory estimation to count indices per IndexSpace (including auxiliary indices).
SeQuant/core/eval/cache_manager.hpp	Adjusts documentation to match generalized AsyCost semantics.
tests/unit/test_asy_cost.cpp	Refactors tests for new API and adds coverage for >2 spaces and missing extents behavior.
tests/unit/test_eval_node.cpp	Updates expected costs to the new API and adds tests for density-fitting, batching, and MR-like examples.

---

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

[evaleev]

Review: Generalize AsyCost

Overview

This PR replaces AsyCost's hard-coded two-space (occupied O / virtual V) cost representation with a general IndexSpace → exponent map. A cost term becomes a rational prefactor times a product of arbitrary index-space sizes raised to integer powers. The API changes accordingly: constructors take an ExponentMap, ops() takes an ExtentMap of per-space sizes, printing uses each space's base_key(), and ordering compares total polynomial degree first, then space-by-space. eval_node.hpp is rewritten to tally indices per IndexSpace (now including aux slots, so DF/THC auxiliary spaces show up), and tests are migrated plus extended with multi-space, density-fitting, batched-index, and MRCC cases.

This is a clean, well-motivated generalization. Code quality is high, documentation is thorough, and test coverage is genuinely improved. Points worth discussing below.

Correctness

The new ordering loses the implicit V≫O weighting — please confirm this is intended. The old operator< compared the virtual exponent first, then occupied. Since V is typically much larger than O, that ordering tracked true magnitude reasonably well. The new ordering sorts by total degree (sum of exponents) first. These disagree whenever total degrees differ:

• O²V⁴ (degree 6) vs O⁵V² (degree 7): old ranks O²V⁴ greater; new ranks O⁵V² greater. With V≫O, O²V⁴ is the larger cost, so the new ordering disagrees with actual magnitude.

This is defensible for a space-agnostic representation (you can't weight spaces without their sizes), and the same-degree tie-break by IndexSpace priority reproduces the old virt-dominant behavior within a degree. The key mitigant — which I verified — is that AsyCost is analysis/reporting only: it is consumed solely by Flops/Memory/min_storage/asy_cost in eval_node.hpp and by peak_cache (std::max(curr, max) in cache_manager.cpp). The optimize module does not use AsyCost for path selection, so contraction-order decisions are unaffected. Worth a one-line note in the PR/commit that this changes how peak_cache and reported costs rank, even though no optimizer decision depends on it.

ops() dropped the temp > 1 ? … : 0 guard — minor behavior change. The old code skipped a term whose substituted value was ≤ 1; the new loop always adds prefactor * temp. Since entries with empty exponents collapse to zero and never persist, the only divergence is when an extent of 0 or 1 is supplied (pow(1, e) = 1), which previously zeroed the term and now keeps it. Harmless for realistic extents (≥10) and arguably more correct — just flagging it.

Batched-index logic is correct. ContractedIndexCount now dedupes all indices across L/R/T into a set and counts distinct indices per space. A contracted index (in both operands) and an external index (operand + result) each collapse to one entry; a batched index (in L, R and T) also collapses to one, yielding the right a³z¹ scaling. The batched index test confirms this. 👍

Code Quality

• Excellent doc comments on the new API and the ordering rationale; the space_counts note about union-space indices (MR hole I=i∪u counted as a single symbol, not decomposed) is exactly the kind of subtlety worth recording.
• Moving the helpers from an anonymous namespace into detail and fixing the is_outerpod → is_outerprod typo are nice cleanups.
• Dead code: ContractedIndexCount::rank(NodePos) now has no consumers — Flops only uses unique_counts() and is_outerprod(). ranks_ is still needed internally for the outer-product check, but the public rank() getter can be dropped.
• Micro-nit: the zero-exponent purge in the constructor could be std::erase_if(exponents_, [](auto& kv){ return kv.second == 0; }), though the manual loop is fine.

Tests

Strong coverage: multi-space (>2) printing/equality/eval, extent fallback to approximate_size(), default-empty ops(), total-degree vs priority ordering, DF factorization, batched indices, and an MRCC active-space case. Two small suggestions:

• The generalized spaces LaTeX assertions check substring presence but not term order. Since ordering is a core behavior change, consider one assertion on the full rendered string to pin down term sequence.
• No test exercises AsyCost::max() flowing through ops() (should return infinity) or through the new is_max branch of operator<. Cheap to add and the new is_max_ sentinel logic is otherwise only indirectly covered.

Risk

Low. The blast radius is contained to eval cost reporting; all production call sites (eval_node.hpp, cache_manager) are updated in this PR and no external callers of the removed AsyCost(size_t,size_t) / ops(size_t,size_t) overloads remain. The main thing to consciously sign off on is the total-degree-first ordering as the intended new semantics for peak_cache/reported costs.

Recommendation

Approve with minor follow-ups: (1) note the ordering-semantics change in the commit/PR description, (2) drop the now-unused rank() getter, (3) optionally add max()-through-ops() and term-order tests.

[evaleev]

good to merge once checks pass