Opened 2 years ago
Closed 21 months ago
#31137 closed defect (fixed)
Fix subs failure involving integervalued rational exponents
Reported by:  Emmanuel Charpentier  Owned by:  

Priority:  minor  Milestone:  sage9.3 
Component:  symbolics  Keywords:  factor 
Cc:  Samuel Lelièvre  Merged in:  
Authors:  Dave Morris  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  c2b45c0 (Commits, GitHub, GitLab)  Commit:  c2b45c0b3181ff8803e116e4c92ecf4c4bd748d9 
Dependencies:  #30786, #30446  Stopgaps: 
Description (last modified by )
From Ask Sage question 54986:
subs
fails with a memory error (or doesn't return) on some expressions.
The following works fine:
sage: _ = var('A, L, G, R, f, k, n, q, u, beta, gamma', domain="positive") sage: a = I*R^2*f^3*k*q*A*u sage: b = 2*pi*L*R^2*G*f^4*k^2*q  2*pi*L*R^2*G*f^4*q  2*pi*L*R^2*beta^2*G*q sage: c = (2*I*pi*L*R^2*beta*gamma*q + 2*I*pi*L*R*(beta + q))*G*f^3 sage: d = 2*(pi*(beta^2 + 1)*L*R^2*q + pi*L*R*beta*gamma*q + pi*L*beta)*G*f^2 sage: e = (2*I*pi*L*R^2*beta*gamma*q  2*I*pi*(beta^2*q + beta)*L*R)*G*f sage: expr = a / ((b + c + d + e)*n) sage: R1 = (expr.real()^2 + expr.imag()^2).factor() sage: R2 = (sqrt(expr.real()^2 + expr.imag()^2)^2).factor() sage: R3 = ((sqrt(expr.real()^2 + expr.imag()^2).factor())^2).factor() sage: all((R1 == R2, R1 == R3, R2 == R3)) True
These expressions have the same string representation:
sage: [len(str(ex)) for ex in (R1, R2, R3)] [734, 734, 734] sage: all((str(R1) == str(R2), str(R1) == str(R3), str(R2) == str(R3)))
Substituting f = 2*beta
in R1
R2
R3
works:
sage: R1s = R1.subs(f = 2*beta) sage: R2s = R2.subs(f = 2*beta) sage: R3s = R3.subs(f = 2*beta)
However, the following never return:
sage: bool(R3s == R1s) # hangs
sage: len(str(R1s)) 520 sage: len(str(R2s)) 520 sage: len(str(R3s)) # hangs
Attempting to interrupt the last call can result in a Sage crash; the original poster reports a "Memory Error" exception.
As a comparison, substituting in R3
via SymPy works:
sage: import sympy sage: bool(sympy.sympify(R3).subs(f, 2*beta)._sage_() == R1.subs(f = 2*beta)) True
but seems to fail via Mathematica somehow:
sage: mma = mathematica sage: bool(mma.Replace(R3, mma.Rule(f, 2*beta)).sage() == R1.subs(f = 2*beta)) False
Priority set to critical, because it affects a very basic feature of Sage.
RESOLUTION: solved by the patch to pynac in #30446. There was a bug in evaluating an expression (perhaps sqrt(expr.real())^2
) that has a rational exponent (such as 2/2
) that simplifies to an integer. This ticket just adds a doctest to ensure that the problem is not allowed to return.
Change History (13)
comment:1 Changed 2 years ago by
Cc:  Samuel Lelièvre added 

Description:  modified (diff) 
comment:2 Changed 2 years ago by
Description:  modified (diff) 

Keywords:  factor added 
Seems expressions resulting from factor
behave strangely.
Not sure what role subs
plays here.
comment:3 Changed 2 years ago by
What seemed to be hanging was just taking a long time. Still, the result is surprising.
sage: len(str(R3s)) # long time  1,292,914,505 1292914505
comment:4 Changed 23 months ago by
Branch:  → public/31137 

comment:5 Changed 23 months ago by
Authors:  → Dave Morris 

Commit:  → 5da750a7fca49b55bd131585f0c620490611834b 
Dependencies:  → #30786 
Priority:  critical → minor 
Status:  new → needs_review 
comment:6 Changed 23 months ago by
Dependencies:  #30786 → #30786, #30446 

Status:  needs_review → needs_work 
only the patch for expression.pyx is needed here, with the rest in #30446
comment:7 Changed 23 months ago by
Commit:  5da750a7fca49b55bd131585f0c620490611834b → c2b45c0b3181ff8803e116e4c92ecf4c4bd748d9 

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
f74f66c  sage package updatelatest: Accept package classes :standard:, :optional: etc., restrict to normal Python packages

182b3d2  sage package fixchecksum: Handle package classes, ignore nonnormal packages

9a57cf6  pynac update

563940e  doctest from #30446

51def3c  sane version name

4834dc8  remove upstreamed patch

214712d  tarball update

6ee3113  doctests for trac 28620, 30304, 30786

6161215  fixes for trac #30446

c2b45c0  doctest for trac 31137

comment:8 Changed 23 months ago by
Reviewers:  → Dima Pasechnik 

Status:  needs_work → positive_review 
LGTM
comment:11 Changed 23 months ago by
Description:  modified (diff) 

comment:12 Changed 23 months ago by
Description:  modified (diff) 

Summary:  subs fails in obscure circumstances → Fix subs failure involving integervalued rational exponents 
Thanks for the explanation.
comment:13 Changed 21 months ago by
Branch:  public/31137 → c2b45c0b3181ff8803e116e4c92ecf4c4bd748d9 

Resolution:  → fixed 
Status:  positive_review → closed 
Seems to have a lot to do with
factor
.I also got a crash when interrupting the computation: