Investigate test-pari failure for A131083 periodic sequence #637
+58
−0
Conversation
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
Co-authored-by: ckrause <840744+ckrause@users.noreply.github.com>
The getNumInitialTermsNeeded function was calculating one fewer initial term than required for recurrence relations. For a recurrence a(n) = a(n-k), we need k initial terms, but the formula was producing only k-1 terms. Added +1 to account for the initial state before the loop, ensuring enough terms to avoid undefined references in PARI evaluation. Fixes issue where test-pari fails for A131083 with deep recursion error. Co-authored-by: ckrause <840744+ckrause@users.noreply.github.com>
Investigated the test-pari failure for A131083. The issue is that the generated formula has 7 initial terms when 8 are needed for the recurrence a(n) = a(n-8). Attempted fix: Add +1 to initial terms for counter cell in getNumInitialTermsNeeded. However, this approach doesn't work because the variant simplification system (simplifyFormulaUsingVariants) overwrites the calculated numTerms. The root cause appears to be in how variants calculate their num_initial_terms, which ends up being one less than needed for periodic sequences like A131083. Further investigation needed to fix the variant system or detect periodic sequences specifically. Co-authored-by: ckrause <840744+ckrause@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix test-pari command failure for A131083
Investigate test-pari failure for A131083 periodic sequence
Dec 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The
test-paricommand fails for A131083 with deep recursion error. The generated PARI formula has 7 initial terms but needs 8 for the recurrencea(n) = a(n-8)with offset 1:When evaluating
a(8), it callsa(0)which is undefined, causing infinite recursion.Investigation Summary
The issue stems from interactions between formula generation and variant simplification:
getNumInitialTermsNeeded()computesloopCounterDecrement * stateful.size() = 1 * 7 = 7simplifyFormulaUsingVariants()applies an optimized variant that also specifies 7 terms, overwriting the calculated valuea(8) = 30Attempted Fixes
Root Cause
The variant system recalculates
num_initial_termsfor simplified formulas, but this calculation is off-by-one for pure periodic recurrences. The fix requires either:num_initial_termscalculation logica(n) = a(n-k)with no additional termsTest files added for reproduction. The variant simplification system (lines 199-244 in
src/form/variant.cpp) needs review to understand why it underestimates initial terms for periodic sequences.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.