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

Endless symbolic computation #24883

Closed
sagetrac-ipasquinelli mannequin opened this issue Mar 2, 2018 · 31 comments
Closed

Endless symbolic computation #24883

sagetrac-ipasquinelli mannequin opened this issue Mar 2, 2018 · 31 comments

Comments

@sagetrac-ipasquinelli
Copy link
Mannequin

sagetrac-ipasquinelli mannequin commented Mar 2, 2018

I tried calculating


a=e^(I*pi/4)+1
b=1-e^(I*pi/4)
a*b

and both expressions a*b and a/b don't stop computing.

I tried both on sage-8.1 for Windows and on sage-8.2.beta6 on Ubuntu (native Ubuntu desktop on Windows10).

Depends on #24838

CC: @rwst @videlec

Component: symbolics

Keywords: days93, days94

Author: Irene Pasquinelli

Branch/Commit: 4d0c51e

Reviewer: Ralf Stephan, Travis Scrimshaw

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

@sagetrac-ipasquinelli sagetrac-ipasquinelli mannequin added this to the sage-8.2 milestone Mar 2, 2018
@videlec
Copy link
Contributor

videlec commented Mar 2, 2018

comment:1

Could you add your architecture/sage version in the ticket description? I can confirm the behavior on archlinux with compiled sage-8.2.beta6.

@videlec
Copy link
Contributor

videlec commented Mar 2, 2018

comment:2

Alternative computation that terminates:

UCF = UniversalCyclotomicField()
a = UCF.zeta(8) + 1
b = 1 - UCF.zeta(8)
a * b

@sagetrac-ipasquinelli

This comment has been minimized.

@sagetrac-sdrewitz
Copy link
Mannequin

sagetrac-sdrewitz mannequin commented Mar 2, 2018

comment:4

I can confirm it on archlinux with compiled sage-8.2.beta6, sage-8.2.beta5 and sage-8.1.
However, with the sagemath 8.1-11 from the archlinux community repository it does work.

sage: a = 1 + e^(I*pi/4)
sage: b = 1 - e^(I*pi/4)
sage: a*b
1/4*((I + 1)*sqrt(2) - 2)*(-(I + 1)*sqrt(2) - 2)
sage: a/b
-1/2*(-(I + 1)*sqrt(2) - 2)/(-(1/2*I + 1/2)*sqrt(2) + 1)

@rwst
Copy link

rwst commented Mar 2, 2018

comment:5

Confirmed in beta6. The loop is in Pynac. Thanks for the report.

@rwst
Copy link

rwst commented Mar 2, 2018

comment:6

Actually there were changes in pynac-0.7.17 that appear to have fixed it. With #24838:

sage:  sage: a = 1 + e^(I*pi/4)
....:  sage: b = 1 - e^(I*pi/4)
....:  sage: a*b
....: 
1/4*((I + 1)*sqrt(2) - 2)*(-(I + 1)*sqrt(2) - 2)

We might doctest this in this ticket, though.

@videlec
Copy link
Contributor

videlec commented Mar 2, 2018

comment:7

Replying to @rwst:

Actually there were changes in pynac-0.7.17 that appear to have fixed it. With #24838:

Cool

We might doctest this in this ticket, though.

+1

@videlec
Copy link
Contributor

videlec commented Mar 2, 2018

Author: Irene Pasquinelli

@videlec
Copy link
Contributor

videlec commented Mar 2, 2018

comment:8

Ralf, Irene is working on this ticket (she is learning how to develop). We will have a branch in a minute. Thanks for pointing #24838.

@sagetrac-ipasquinelli
Copy link
Mannequin Author

sagetrac-ipasquinelli mannequin commented Mar 2, 2018

Branch: u/ipasquinelli/24883

@sagetrac-ipasquinelli
Copy link
Mannequin Author

sagetrac-ipasquinelli mannequin commented Mar 2, 2018

New commits:

e470685version / chkum / rm patch
d3511ce24838: doctest fixes
efe5f1424883: adding doctest for symbolic

@sagetrac-ipasquinelli
Copy link
Mannequin Author

sagetrac-ipasquinelli mannequin commented Mar 2, 2018

Commit: efe5f14

@sagetrac-ipasquinelli
Copy link
Mannequin Author

sagetrac-ipasquinelli mannequin commented Mar 2, 2018

Dependencies: #24838

@rwst
Copy link

rwst commented Mar 3, 2018

Reviewer: Ralf Stephan

@rwst
Copy link

rwst commented Mar 3, 2018

comment:11

I think this is fine, but we may have to wait for setting positive until #24838 gets positive.

@rwst
Copy link

rwst commented Jul 1, 2018

Changed branch from u/ipasquinelli/24883 to public/24883

@rwst
Copy link

rwst commented Jul 1, 2018

Changed commit from efe5f14 to 699c0a2

@rwst
Copy link

rwst commented Jul 1, 2018

comment:13

Okay, the dependency #24838 is now in the develop branch. Your branch no longer merges because there were further changes in the dependency. Instead of merging them I created a fresh branch with only your commit (using git cherry-pick).


New commits:

699c0a224883: adding doctest for symbolic

@tscrim
Copy link
Collaborator

tscrim commented Jul 1, 2018

Changed keywords from days93 to days93, days94

@tscrim
Copy link
Collaborator

tscrim commented Jul 1, 2018

Changed reviewer from Ralf Stephan to Ralf Stephan, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 1, 2018

comment:14

LGTM.

@vbraun
Copy link
Member

vbraun commented Jul 5, 2018

comment:15

see patchbot

@tscrim
Copy link
Collaborator

tscrim commented Jul 5, 2018

comment:16

Problem actually comes from an earlier doctest:

sage: e = x+1 <= x-2

(I tested this by copy/pasting the doctest).
So we probably need to use exp.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2018

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

cb24f9a24883: improve usage of ambigous symbol in doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 6, 2018

Changed commit from 699c0a2 to cb24f9a

@videlec
Copy link
Contributor

videlec commented Aug 3, 2018

comment:19

update milestone 8.3 -> 8.4

@videlec videlec modified the milestones: sage-8.3, sage-8.4 Aug 3, 2018
@tscrim
Copy link
Collaborator

tscrim commented Aug 4, 2018

comment:20

Forgot about this. LGTM.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 4, 2018

Changed commit from cb24f9a to 4d0c51e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 4, 2018

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

4d0c51eMerge with sage-8.3

@bryangingechen
Copy link
Mannequin

bryangingechen mannequin commented Aug 4, 2018

comment:22

Replying to @sagetrac-git:

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

4d0c51eMerge with sage-8.3

I fixed a trivial merge conflict with 8.3.

@vbraun
Copy link
Member

vbraun commented Aug 6, 2018

Changed branch from public/24883 to 4d0c51e

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

4 participants