From fecd4ec5ac1417d8eadea6281f7048dc5b23b606 Mon Sep 17 00:00:00 2001 From: Guo Chen Date: Sat, 15 Aug 2026 22:24:31 +0800 Subject: [PATCH 1/2] fix: Restrict the unnatural-partition skip to classical contractions The search skips a partition that shatters the factors into more than two chunks whose subproblems are all already memoized. That is an acceleration for classical tensor contractions, where every summation is involved by exactly two factors. The thesis describes it as such: "code to accelerate the process specifically for classical TC problems where each summation index appears exactly twice". It was applied to every problem. When a summation is involved by more than two factors the skip is not valid, and it fails in two ways. The optimal parenthesization is lost. Over 500 randomly generated contraction problems, the normal mode returned a suboptimal result for 68 of them, always more expensive than the exhaustive mode, never cheaper. All 68 have a summation on more than two factors. Every candidate can be skipped, leaving the subproblem with no evaluation at all. The `assert(!evals.empty())` at the end of `opt` then fires in a debug build, and `evals.front()` is read on an empty vector in a release build, which crashes. The smallest case is a single summation over four factors, `s = sum_i x[i] y[i] z[i] w[i]`. Roughly five per cent of the randomly generated configurations crashed. Gate the skip on the problem being classical, and additionally never let it consume the last candidate, so the read of `evals.front()` can no longer be reached with nothing found. Verified by fingerprinting every intermediate, its parenthesization and its cost across the same 500 problems in all six mode and inclusivity combinations. On classical problems all 1042 configurations are unchanged, so the acceleration is kept exactly where it is sound. The 139 crashing configurations all complete, and the normal mode now agrees with the exhaustive mode on every problem. Co-Authored-By: Claude Opus 5 --- deps/fbitset | 2 +- include/libparenth.hpp | 31 ++++++++++++- test/CMakeLists.txt | 1 + test/nonclassical.cpp | 99 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 test/nonclassical.cpp diff --git a/deps/fbitset b/deps/fbitset index 765b2f7..5efdbb3 160000 --- a/deps/fbitset +++ b/deps/fbitset @@ -1 +1 @@ -Subproject commit 765b2f740ca71e732d047aa65e067c1a9018feee +Subproject commit 5efdbb3208a170e8c2355ecb3caf78aced8895c8 diff --git a/include/libparenth.hpp b/include/libparenth.hpp index f79bb2a..87eaaa9 100644 --- a/include/libparenth.hpp +++ b/include/libparenth.hpp @@ -97,6 +97,7 @@ class Parenther { , n_sums_{ n_sums } , factors_with_{} , dims_on_{} + , if_classical_{ true } { assert(std::is_sorted(dims_.cbegin(), dims_.cbegin() + n_sums_)); @@ -116,6 +117,13 @@ class Parenther { } assert(factor_idx == n_factors); + + for (Size i = 0; i < n_sums_; ++i) { + if (factors_with_[i].count() != 2) { + if_classical_ = false; + break; + } + } } // Except the above constructor, normally we would put the basic @@ -750,7 +758,20 @@ class Parenther { } // Unnatural partition. - if (if_for_opt && chunks.size() > 2 + // + // This skipping is an acceleration for classical contractions, + // where every summation is involved by exactly two factors. There + // a partition shattering the factors into more than two chunks + // that are all already memoized is also reachable from a smaller + // set of broken summations, so nothing is lost by skipping it. + // When a summation is involved by more than two factors that no + // longer holds: the skipping then loses the optimal + // parenthesization, and skipping every candidate would leave the + // subproblem without any evaluation at all, making the read of + // `evals.front()` below undefined. Hence the restriction to + // classical problems, and the guard keeping one evaluation. + if (if_for_opt && if_classical_ && !evals.empty() + && chunks.size() > 2 && std::all_of( chunks.cbegin(), chunks.cend(), [&mem](const Subset& i) { return mem.count(i.factors) != 0; @@ -872,6 +893,14 @@ class Parenther { */ std::vector dims_on_; + + /** If every summation is involved by exactly two factors. + * + * This is the classical tensor contraction problem. Some of the + * accelerations of the search are only valid under this assumption. + */ + + bool if_classical_; }; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index dd78f0b..5fb6814 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -12,6 +12,7 @@ FetchContent_MakeAvailable(Catch2) # The main test driver. add_executable(testmain matrixchain.cpp + nonclassical.cpp ) target_link_libraries(testmain PRIVATE Catch2::Catch2WithMain) diff --git a/test/nonclassical.cpp b/test/nonclassical.cpp new file mode 100644 index 0000000..d45d3ab --- /dev/null +++ b/test/nonclassical.cpp @@ -0,0 +1,99 @@ +/** Tests on contractions where a summation is involved by more than two + * factors. + * + * A classical tensor contraction has every summation involved by exactly two + * factors. The search has an acceleration that skips a partition shattering + * the factors into more than two already-memoized chunks, which is only valid + * under that assumption. These tests cover problems outside it. + */ + +#include +#include + +#include + +// Disable Catch2's range detection for fbitset by providing stream insertion +// operator. +namespace fbitset { +template +inline std::ostream& operator<<(std::ostream& os, const Fbitset& fs) +{ + os << "Fbitset<" << N << ">{count=" << fs.count() << "}"; + return os; +} +} + +#include + +using namespace libparenth; + +using Dim = size_t; +using P = Parenther; + +/** Reads the cost of the optimal evaluation of the whole problem. + */ + +static Dim top_cost(const P::Mem& res, size_t n_factors) +{ + P::Factor_subset all(n_factors, true); + auto it = res.find(all); + REQUIRE(it != res.end()); + REQUIRE(!it->second.evals.empty()); + return it->second.evals.front().cost; +} + +TEST_CASE("A summation over four factors can be parenthesized") +{ + // s = sum_i x[i] y[i] z[i] w[i], with no external index at all. Every + // bipartition of the four factors breaks the single summation, so the + // partitions shatter into four chunks. Before the acceleration was + // restricted to classical problems, every candidate was skipped here and + // the subproblem was left without any evaluation. + std::vector dims = { 100 }; + std::vector> factors = { { 0 }, { 0 }, { 0 }, { 0 } }; + + P parenther( + dims.cbegin(), dims.cend(), 1, factors.cbegin(), factors.cend()); + + SECTION("The greedy strategy terminates with an evaluation") + { + auto res = parenther.opt(Mode::GREEDY, false); + CHECK(top_cost(res, 4) > 0); + } + + SECTION("The optimal strategy terminates with an evaluation") + { + auto res = parenther.opt(Mode::NORMAL, false); + CHECK(top_cost(res, 4) > 0); + } + + SECTION("All strategies agree on the optimal cost") + { + P p_normal( + dims.cbegin(), dims.cend(), 1, factors.cbegin(), factors.cend()); + P p_exhaust( + dims.cbegin(), dims.cend(), 1, factors.cbegin(), factors.cend()); + + auto normal = top_cost(p_normal.opt(Mode::NORMAL, false), 4); + auto exhaust = top_cost(p_exhaust.opt(Mode::EXHAUST, false), 4); + CHECK(normal == exhaust); + } +} + +TEST_CASE("A shared summation does not hide the optimal parenthesization") +{ + // t[e] = sum_{i, j} a[i, j, e] b[i, j] c[i, j] d[j]. Both summations are + // involved by more than two factors, so the optimal parenthesization is + // only reachable through a partition that the acceleration used to skip. + std::vector dims = { 10, 20, 30 }; + std::vector> factors + = { { 0, 1, 2 }, { 0, 1 }, { 0, 1 }, { 1 } }; + + P p_normal(dims.cbegin(), dims.cend(), 2, factors.cbegin(), factors.cend()); + P p_exhaust( + dims.cbegin(), dims.cend(), 2, factors.cbegin(), factors.cend()); + + auto normal = top_cost(p_normal.opt(Mode::NORMAL, false), 4); + auto exhaust = top_cost(p_exhaust.opt(Mode::EXHAUST, false), 4); + CHECK(normal == exhaust); +} From 571b552f16602b9e1cf3eb18076dfca0d06f28a5 Mon Sep 17 00:00:00 2001 From: Guo Chen Date: Sun, 16 Aug 2026 00:17:45 +0800 Subject: [PATCH 2/2] deps: Point fbitset back at master fbitset #6 is merged, so the temporary pin at that branch can go. It was rebased on merge, so the branch commit is not on master and the pin would have dangled once the branch is deleted. The header content at 0a21596 is identical to what was pinned. Tests pass in both the debug and the release build. Co-Authored-By: Claude Opus 5 --- deps/fbitset | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/fbitset b/deps/fbitset index 5efdbb3..0a21596 160000 --- a/deps/fbitset +++ b/deps/fbitset @@ -1 +1 @@ -Subproject commit 5efdbb3208a170e8c2355ecb3caf78aced8895c8 +Subproject commit 0a21596ccbcb165237b0070a3b93584839ce1a21