Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actually fix: "Do not use pynac's poly_mul_expand function until it has been debugged" #31679

Closed
mkoeppe opened this issue Apr 17, 2021 · 8 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Apr 17, 2021

#31479 with this title was merged in 9.3.rc3 but its branch contained something else.

CC: @DaveWitteMorris

Component: symbolics

Author: Dave Morris

Branch/Commit: 2b88eb5

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/31679

@mkoeppe mkoeppe added this to the sage-9.3 milestone Apr 17, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 17, 2021

Author: Dave Morris

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 17, 2021

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Apr 18, 2021

comment:4

On 32-bit:

**********************************************************************
File "src/sage/symbolic/expression.pyx", line 4882, in sage.symbolic.expression.Expression.expand
Failed example:
    ((a + b + c)^30 * (3*b + d - 5/d)^3).expand().subs(a=0,b=2,c=-1)
Expected:
    d^3 + 18*d^2 + 93*d - 465/d + 450/d^2 - 125/d^3 + 36  
Got:
    d^3 + 18*d^2 + 1739461754973*d - 8697308774865/d + 450/d^2 - 125/d^3 + 36
**********************************************************************
1 item had failures:
   1 of  43 in sage.symbolic.expression.Expression.expand
    [2941 tests, 1 failure, 44.35 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/symbolic/expression.pyx  # 1 doctest failed
----------------------------------------------------------------------

@DaveWitteMorris
Copy link
Member

comment:5

I don't have time to push a branch until tomorrow, but I think that (for 9.3) we can solve the problem by weakening the doctest so that it works for both 64-bit and 32-bit. Maybe:

sage: a,b,c,d = var("a b c d")
sage: f = ((a + b + c)^30 * (3*b + d - 5/d)^3).expand().subs(a=0,b=2,c=-1)
sage: sum(sign(s) * (abs(ZZ(s)) % ZZ(2^20)) * d^i for s,i in f.coefficients())
d^3 + 18*d^2 + 93*d - 465/d + 450/d^2 - 125/d^3 + 36

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

ef84adfMerge branch 'public/31554' of git://trac.sagemath.org/sage into #31679polymulexpand
2b88eb5fix doctest for 32-bit

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2021

Changed commit from 296d4c7 to 2b88eb5

@mkoeppe
Copy link
Member Author

mkoeppe commented Apr 19, 2021

comment:8

This doctest looks a bit strange, but overall this is good enough as a hotfix for 9.3 for this severe bug.

@vbraun
Copy link
Member

vbraun commented Apr 24, 2021

Changed branch from public/31554 to 2b88eb5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants