From 3fdefb03c6f9c4b09f107a3f84a40f2ebdb01df0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 20:36:29 +0000 Subject: [PATCH 1/4] Initial plan From f7d1f9eb853e4378c925db22cd02a22802b1e20e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 28 Nov 2025 20:54:33 +0000 Subject: [PATCH 2/4] Fix PARI formula generation for rising factorial when base can be zero Co-authored-by: ckrause <840744+ckrause@users.noreply.github.com> --- CHANGELOG.md | 1 + src/form/formula_gen.cpp | 68 +++++++++++++++++++---------- tests/formula/formula.txt | 1 + tests/formula/pari-function.txt | 1 + tests/programs/oeis/006/A006430.asm | 36 +++++++++++++++ 5 files changed, 83 insertions(+), 24 deletions(-) create mode 100644 tests/programs/oeis/006/A006430.asm diff --git a/CHANGELOG.md b/CHANGELOG.md index 00d83c8f..51a4acfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Bugfixes * Fix parsing error during stats generation +* Fix PARI formula generation for rising factorial when the base can be zero (e.g., A006430) ### Enhancements diff --git a/src/form/formula_gen.cpp b/src/form/formula_gen.cpp index dfd3295d..793cccbb 100644 --- a/src/form/formula_gen.cpp +++ b/src/form/formula_gen.cpp @@ -124,15 +124,16 @@ Expression FormulaGenerator::cellFunc(int64_t index) const { return ExpressionUtil::newFunction(getCellName(index)); } +// Maximum size of product expansion for factorial +constexpr int64_t MAX_PRODUCT_EXPANSION = 10; + // Express falling/rising factorial using standard factorial bool FormulaGenerator::facToExpression(const Expression& a, const Expression& b, const Operand& aOp, const Operand& bOp, const RangeMap* ranges, Expression& res) const { - - // Optimize for small constants: convert to PRODUCT instead of FACTORIAL - if (b.type == Expression::Type::CONSTANT && b.value >= Number(-3) && - b.value <= Number(3)) { + // Check if b is a constant + if (b.type == Expression::Type::CONSTANT) { auto k = b.value.asInt(); // Trivial case: k = 0 -> result is 1 @@ -147,37 +148,55 @@ bool FormulaGenerator::facToExpression(const Expression& a, const Expression& b, return true; } - // Build product expression - res = Expression(Expression::Type::PRODUCT); - if (k > 0) { - // Rising factorial: a * (a+1) * (a+2) * ... * (a+k-1) - for (int64_t i = 0; i < k; i++) { - if (i == 0) { - res.newChild(a); - } else { - res.newChild(sum({a, constant(i)})); - } + // For small constants or when factorial division would fail, + // use product expansion + bool useProductExpansion = (k >= -MAX_PRODUCT_EXPANSION && k <= MAX_PRODUCT_EXPANSION); + + // For rising factorial, check if we can safely use factorial division + // The denominator is (a-1)!, which requires a >= 1 + if (!useProductExpansion && k > 0) { + Expression denomArg = sum({a, constant(-1)}); + ExpressionUtil::normalize(denomArg); + if (ExpressionUtil::canBeNegative(denomArg, offset)) { + // Factorial division would fail, use product expansion instead + useProductExpansion = true; } - } else { - // Falling factorial: a * (a-1) * (a-2) * ... * (a+k+1) - // Note: k is negative, so a+k+1 < a - for (int64_t i = 0; i < -k; i++) { - if (i == 0) { - res.newChild(a); - } else { - res.newChild(sum({a, constant(-i)})); + } + + if (useProductExpansion) { + // Build product expression + res = Expression(Expression::Type::PRODUCT); + if (k > 0) { + // Rising factorial: a * (a+1) * (a+2) * ... * (a+k-1) + for (int64_t i = 0; i < k; i++) { + if (i == 0) { + res.newChild(a); + } else { + res.newChild(sum({a, constant(i)})); + } + } + } else { + // Falling factorial: a * (a-1) * (a-2) * ... * (a+k+1) + // Note: k is negative, so a+k+1 < a + for (int64_t i = 0; i < -k; i++) { + if (i == 0) { + res.newChild(a); + } else { + res.newChild(sum({a, constant(-i)})); + } } } + return true; } - return true; } + // TODO: can we relax the negativity check for b? if (canBeNegativeWithRanges(a, aOp, ranges) || canBeNegativeWithRanges(b, bOp, ranges)) { return false; } - // General case + // General case: use factorial division Expression num(Expression::Type::FACTORIAL); Expression denom(Expression::Type::FACTORIAL); @@ -191,6 +210,7 @@ bool FormulaGenerator::facToExpression(const Expression& a, const Expression& b, } else { // rising factorial // (a+b-1)!/(a-1)! + // Note: we already checked that a-1 >= 0 above for small constants num.children = {sum({a, sum({b, constant(-1)})})}; denom.children = {sum({a, constant(-1)})}; } diff --git a/tests/formula/formula.txt b/tests/formula/formula.txt index a3507da7..eea92521 100644 --- a/tests/formula/formula.txt +++ b/tests/formula/formula.txt @@ -77,6 +77,7 @@ A005408: a(n) = 2*n+1 A005600: a(n) = (3*min(n-3,6)-((n%4)==0)+1)%10 A005843: a(n) = 2*n A006221: a(n) = n*(n*(34*n+51)+27)+5 +A006430: a(n) = truncate((floor(((-(1==n)+n)*(-(1==n)+n+1)*(-(1==n)+n+2)*(-(1==n)+n+3))/(-(1==n)+n+1))*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*(23*n-23*(1==n)+963)+17544)+147952)+481675)-1052153)-7850914)-2900162)+60869272)+37067400)-179920800))/79833600) A007395: a(n) = 2 A007531: a(n) = n*(n-2)*(n-1) A007583: a(n) = 2*floor((4^n)/3)+1 diff --git a/tests/formula/pari-function.txt b/tests/formula/pari-function.txt index b0f337f4..b2852522 100644 --- a/tests/formula/pari-function.txt +++ b/tests/formula/pari-function.txt @@ -5,3 +5,4 @@ A000058: (a(n) = b(n)+1); (b(n) = if(n==0,1,local(l1=b(n-1)); l1*(l1+1))) A000247: a(n) = 2^n-n-2 A001286: a(n) = floor(((n-1)*n!)/2) A001715: (a(n) = truncate(b(n)/6)); (b(n) = if(n==0,1,n*b(n-1))) +A006430: a(n) = truncate((floor(((-(1==n)+n)*(-(1==n)+n+1)*(-(1==n)+n+2)*(-(1==n)+n+3))/(-(1==n)+n+1))*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*((-(1==n)+n)*(23*n-23*(1==n)+963)+17544)+147952)+481675)-1052153)-7850914)-2900162)+60869272)+37067400)-179920800))/79833600) diff --git a/tests/programs/oeis/006/A006430.asm b/tests/programs/oeis/006/A006430.asm new file mode 100644 index 00000000..29ba57fc --- /dev/null +++ b/tests/programs/oeis/006/A006430.asm @@ -0,0 +1,36 @@ +; A006430: Number of loopless tree-rooted planar maps with 5 vertices and n faces and no isthmuses. +; 0,5,360,7350,73700,474588,2292790,9046807,30676440,92393015,252872984,639382605,1512137536,3377126024,7176513960,14599539314,28575632350,54036739617,99069119952,176618150000,306965183268,521265871700,866527603370,1412513294049,2261193633300,3559534488211,5516605536480,8426235973615,12696729614240,18889491891600,27768822919308,40365602495540,58058144150922,82674136440925,116618332018840,163031500997274,225987148221380,310733619012236,423990500605854,574309684127035,772513103084624 + +#offset 1 + +mov $3,1 +equ $3,$0 +sub $0,$3 +mov $1,$0 +mov $2,$0 +add $2,1 +fac $0,4 +div $0,$2 +sub $2,1 +mul $1,23 +add $1,963 +mul $1,$2 +add $1,17544 +mul $1,$2 +add $1,147952 +mul $1,$2 +add $1,481675 +mul $1,$2 +sub $1,1052153 +mul $1,$2 +sub $1,7850914 +mul $1,$2 +sub $1,2900162 +mul $1,$2 +add $1,60869272 +mul $1,$2 +add $1,37067400 +mul $1,$2 +sub $1,179920800 +mul $0,$1 +div $0,79833600 From 94f1e1a37c72537ea0c35147130580ec2d597113 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 29 Nov 2025 15:46:06 +0000 Subject: [PATCH 3/4] Add range check before Number.asInt() call in facToExpression Co-authored-by: ckrause <840744+ckrause@users.noreply.github.com> --- src/form/formula_gen.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/form/formula_gen.cpp b/src/form/formula_gen.cpp index 793cccbb..8034e75b 100644 --- a/src/form/formula_gen.cpp +++ b/src/form/formula_gen.cpp @@ -132,8 +132,10 @@ bool FormulaGenerator::facToExpression(const Expression& a, const Expression& b, const Operand& aOp, const Operand& bOp, const RangeMap* ranges, Expression& res) const { - // Check if b is a constant - if (b.type == Expression::Type::CONSTANT) { + // Check if b is a constant within a safe range for conversion to int + if (b.type == Expression::Type::CONSTANT && + b.value >= Number(-MAX_PRODUCT_EXPANSION) && + b.value <= Number(MAX_PRODUCT_EXPANSION)) { auto k = b.value.asInt(); // Trivial case: k = 0 -> result is 1 @@ -150,11 +152,11 @@ bool FormulaGenerator::facToExpression(const Expression& a, const Expression& b, // For small constants or when factorial division would fail, // use product expansion - bool useProductExpansion = (k >= -MAX_PRODUCT_EXPANSION && k <= MAX_PRODUCT_EXPANSION); + bool useProductExpansion = true; // For rising factorial, check if we can safely use factorial division // The denominator is (a-1)!, which requires a >= 1 - if (!useProductExpansion && k > 0) { + if (k > 0) { Expression denomArg = sum({a, constant(-1)}); ExpressionUtil::normalize(denomArg); if (ExpressionUtil::canBeNegative(denomArg, offset)) { From 3b2fad4e05a1d174c7b55e4de0c8914d97f40a16 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 29 Nov 2025 15:48:43 +0000 Subject: [PATCH 4/4] Simplify facToExpression logic and improve comments Co-authored-by: ckrause <840744+ckrause@users.noreply.github.com> --- src/form/formula_gen.cpp | 57 +++++++++++++++------------------------- 1 file changed, 21 insertions(+), 36 deletions(-) diff --git a/src/form/formula_gen.cpp b/src/form/formula_gen.cpp index 8034e75b..6c897625 100644 --- a/src/form/formula_gen.cpp +++ b/src/form/formula_gen.cpp @@ -132,7 +132,8 @@ bool FormulaGenerator::facToExpression(const Expression& a, const Expression& b, const Operand& aOp, const Operand& bOp, const RangeMap* ranges, Expression& res) const { - // Check if b is a constant within a safe range for conversion to int + // Check if b is a constant within the product expansion threshold. + // This range check also ensures safe conversion to int64_t. if (b.type == Expression::Type::CONSTANT && b.value >= Number(-MAX_PRODUCT_EXPANSION) && b.value <= Number(MAX_PRODUCT_EXPANSION)) { @@ -150,46 +151,30 @@ bool FormulaGenerator::facToExpression(const Expression& a, const Expression& b, return true; } - // For small constants or when factorial division would fail, - // use product expansion - bool useProductExpansion = true; - - // For rising factorial, check if we can safely use factorial division - // The denominator is (a-1)!, which requires a >= 1 + // Use product expansion for small constants. + // This avoids factorial division issues when a can be zero. + res = Expression(Expression::Type::PRODUCT); if (k > 0) { - Expression denomArg = sum({a, constant(-1)}); - ExpressionUtil::normalize(denomArg); - if (ExpressionUtil::canBeNegative(denomArg, offset)) { - // Factorial division would fail, use product expansion instead - useProductExpansion = true; - } - } - - if (useProductExpansion) { - // Build product expression - res = Expression(Expression::Type::PRODUCT); - if (k > 0) { - // Rising factorial: a * (a+1) * (a+2) * ... * (a+k-1) - for (int64_t i = 0; i < k; i++) { - if (i == 0) { - res.newChild(a); - } else { - res.newChild(sum({a, constant(i)})); - } + // Rising factorial: a * (a+1) * (a+2) * ... * (a+k-1) + for (int64_t i = 0; i < k; i++) { + if (i == 0) { + res.newChild(a); + } else { + res.newChild(sum({a, constant(i)})); } - } else { - // Falling factorial: a * (a-1) * (a-2) * ... * (a+k+1) - // Note: k is negative, so a+k+1 < a - for (int64_t i = 0; i < -k; i++) { - if (i == 0) { - res.newChild(a); - } else { - res.newChild(sum({a, constant(-i)})); - } + } + } else { + // Falling factorial: a * (a-1) * (a-2) * ... * (a+k+1) + // Note: k is negative, so a+k+1 < a + for (int64_t i = 0; i < -k; i++) { + if (i == 0) { + res.newChild(a); + } else { + res.newChild(sum({a, constant(-i)})); } } - return true; } + return true; } // TODO: can we relax the negativity check for b?