Skip to content

Fix #475 and #476: odd-power sqrt fold and permute index validation - #479

Open
petlenz wants to merge 2 commits into
fix-336-pow-pow-domainfrom
fix-475-476-pow-permute
Open

petlenz wants to merge 2 commits into
fix-336-pow-pow-domainfrom
fix-475-476-pow-permute

Conversation

@petlenz

@petlenz petlenz commented Sep 18, 2026

Copy link
Copy Markdown
Member

Closes #475.
Closes #476.

Base is fix-336-pow-pow-domain (PR #462), not main — #475 relaxes the gate that PR adds, so this merges after it.

#475 — pow(sqrt(x), n) folds for an odd n. An odd n leaves a denominator of 2 in n/2, so pow(x, n/2) is non-real for x < 0 exactly where sqrt(x) is; the two sides agree for every real x. An even n would make the right side finite there, so it still requires a nonnegative radicand.

Expression Before Now
pow(sqrt(x), 3), unassumed x pow(sqrt(x),3) pow(x,3/2)
pow(sqrt(x), 2), unassumed x pow(sqrt(x),2) unchanged — correctly blocked
pow(sqrt(x), -3) unfolded pow(x,-3/2)

Numerically verified at x = −4, −0.5, 0, 0.5, 4 for both parities, and for n = -3 / n = -2: folded results match std::pow(std::sqrt(x), n) everywhere, including NaN for negative x and inf at x = 0.

Spelling variants (3, 3.0, 6/2) all fold and evaluate identically. Note the resulting exponent's representation still differs (3/2 vs 1.5) on this base, so the test compares by value; #459/#473 are what make numerically equal spellings one value.

Test change, called out explicitly. Scalar_PowSqrtSimplification asserted pow(sqrt(x), _3) → "pow(sqrt(x),3)". That assertion was introduced by #462's blanket gate; on main it reads "pow(x,3/2)". This restores the original for the odd case and keeps #462's new even-case assertion, with a comment stating the parity condition.

#476 — permutation validated in the node constructor. Only permute_indices() checked that the index sequence covers the operand's rank, so a direct construction could build a node claiming to be a rank-2 transpose while wrapping a rank-4 tensor, which is_trans_of accepted. The check moved into a shared validate_permutation helper that the constructor and the factory both call, so rebuild, substitution and direct construction are covered — the same precedent #438 set for the inner/outer product wrappers. The factory's duplicated loop is gone and its messages are unchanged.

Internal direct constructions audited, all pass validated sequences: tensor_rebuild_visitor.h:112 (reuses the node's own indices), the composition fold and the factory tail in tensor_functions.h, and trans() (rank-2 gated, {2,1}).

Since the malformed node can no longer be constructed, "is_trans_of cannot be fooled" is now enforced at construction rather than asserted separately.

Tests: ScalarEval.PowOfSqrtFoldsForOddExponents and TensorShapeValidation.PermuteWrapperCtorValidatesIndices (mismatched rank, out-of-range, duplicate, plus valid rank-2 and rank-4 permutations still constructing).

Negative control (both production files restored from the base with git show, rebuilt clean): all three of PowOfSqrtFoldsForOddExponents, PermuteWrapperCtorValidatesIndices and the updated Scalar_PowSqrtSimplification fail; all pass with the fixes.

Full suite 2414/2414 (gcc-14 Debug) and 2600/2600 parser-enabled; clang-format clean.

An odd n leaves a denominator of 2 in n/2, so pow(x, n/2) is non-real for
x < 0 exactly where sqrt(x) is, and the two sides agree for every real x.
An even n would make the right side finite there, so it still requires a
nonnegative radicand.

Restores the odd-exponent assertion the blanket gate had replaced.

Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
Only the factory checked that the index sequence covers the operand's rank,
so a direct construction could build a node claiming to be a rank-2
transpose while wrapping a rank-4 tensor — is_trans_of accepted it. The
check moves into a shared helper the ctor and the factory both use, so
rebuild, substitution and direct construction are covered.

Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant