Fix #346: fold duplicate mul factors instead of throwing - #400
Merged
Conversation
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 25, 2026 12:09
150c225 to
3e626cc
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 25, 2026 12:11
5c5b314 to
ce0ac03
Compare
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 25, 2026 13:25
3e626cc to
9af5448
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 25, 2026 13:26
ce0ac03 to
d8d0c05
Compare
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 25, 2026 22:30
42da614 to
9121a68
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 25, 2026 22:33
d8d0c05 to
4cd6afb
Compare
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 26, 2026 12:14
9121a68 to
a4eff94
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 26, 2026 12:14
4cd6afb to
4e982ab
Compare
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 26, 2026 12:43
a4eff94 to
b731e94
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
2 times, most recently
from
July 26, 2026 17:18
acdea06 to
b96ddbc
Compare
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 26, 2026 17:18
b731e94 to
8e05846
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 26, 2026 17:42
b96ddbc to
8d208df
Compare
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 26, 2026 17:42
8e05846 to
352c37e
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 26, 2026 18:16
8d208df to
8003b27
Compare
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
2 times, most recently
from
July 26, 2026 18:40
b20178b to
517baa9
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 26, 2026 18:40
8003b27 to
2d4cbd1
Compare
mul_dispatch::dispatch(mul_type) pushed the LHS factor into a copy of the RHS product unconditionally; n_ary_tree::insert_hash throws on duplicate keys, so sin(x) * (sin(x)*y) crashed with 'internal_error: duplicate child insertion' instead of producing sin(x)^2*y. Symbol/mul/pow LHS types are intercepted by domain dispatchers; function-type factors reached this fallback. n_ary_tree gains merge_or_insert_mul (the *-combining sibling of merge_or_insert, exact keys only) and the dispatch uses it; a duplicate factor now combines via the exponent-addition rule, and a chained collision (pow result meeting an existing pow factor) converges the same way. Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
The merge_or_insert_mul fix covered only x * (x*y); the mirrored order (sin(x)*y) * sin(x) and the mul*mul factor chain (sin(x)*y) * (sin(x)*z) still threw 'duplicate child insertion' through the generic mul-LHS dispatch's bare push_back - multiplication was commutative in value but not in outcome. That dispatch now routes through merge_or_insert_mul too (the mul*mul chain re-enters it per factor, so one fix covers both probes). Also corrects the misleading 'chained collision' test comment: sin(x) and pow(sin(x),2) are distinct exact keys and deliberately coexist (pow-base folding is epic #379's scope). Signed-off-by: petlenz <peterlenz89.pl@gmail.com>
petlenz
force-pushed
the
fix-342-343-index-sequence-identity
branch
from
July 26, 2026 19:03
517baa9 to
df50715
Compare
petlenz
force-pushed
the
fix-346-mul-duplicate-factor
branch
from
July 26, 2026 19:03
2d4cbd1 to
64eb402
Compare
petlenz
changed the base branch from
fix-342-343-index-sequence-identity
to
main
July 30, 2026 06:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #346. Stacked on #399.
sin(x) * (sin(x) * y)threwinternal_error: n_ary_tree::insert_hash: duplicate child insertioninstead of producingsin(x)²·y— the genericmul_dispatch::dispatch(mul_type)fallback pushed the LHS factor unconditionally, and function-type factors (not intercepted by the symbol/mul/pow domain dispatchers) hit the duplicate guard.Design
n_ary_tree::merge_or_insert_mul— the*-combining sibling ofmerge_or_insert(exact keys only; mul factor maps have no coefficient-bearing like terms) — replaces the barepush_back. A duplicate factor combines via the exponent-addition rule (sin(x)·sin(x) → pow(sin(x),2)), and chained collisions (the pow result meeting an existing pow factor) converge through the same loop.Tests
Evaluation-verified lock-in for
sin/logfactors plus the chained-collision case. Full suite: 2431/2431 pass. gcc-14-Werrorcheck clean.