Opened 3 years ago

Closed 3 years ago

#24883 closed defect (fixed)

Endless symbolic computation

Reported by: ipasquinelli Owned by:
Priority: major Milestone: sage-8.4
Component: symbolics Keywords: days93, days94
Cc: rws, vdelecroix Merged in:
Authors: Irene Pasquinelli Reviewers: Ralf Stephan, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 4d0c51e (Commits, GitHub, GitLab) Commit: 4d0c51eab2f477499b6f7c64b5e7d179f8063cf2
Dependencies: #24838 Stopgaps:

Status badges

Description (last modified by ipasquinelli)

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).

Change History (23)

comment:1 Changed 3 years ago by vdelecroix

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

comment:2 Changed 3 years ago by vdelecroix

Alternative computation that terminates:

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

comment:3 Changed 3 years ago by ipasquinelli

  • Description modified (diff)

comment:4 Changed 3 years ago by sdrewitz

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)

comment:5 Changed 3 years ago by rws

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

comment:6 follow-up: Changed 3 years ago by rws

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.

comment:7 in reply to: ↑ 6 Changed 3 years ago by vdelecroix

Replying to rws:

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

comment:8 Changed 3 years ago by vdelecroix

  • Authors set to Irene Pasquinelli

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.

comment:9 Changed 3 years ago by ipasquinelli

  • Branch set to u/ipasquinelli/24883
  • Commit set to efe5f145b805b60d72a12eaf9c57a5109e0c786a
  • Status changed from new to needs_review

New commits:

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

comment:10 Changed 3 years ago by ipasquinelli

  • Dependencies set to #24838

comment:11 Changed 3 years ago by rws

  • Reviewers set to Ralf Stephan

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

comment:12 Changed 3 years ago by rws

  • Branch changed from u/ipasquinelli/24883 to public/24883

comment:13 Changed 3 years ago by rws

  • Commit changed from efe5f145b805b60d72a12eaf9c57a5109e0c786a to 699c0a2215c0b2472a70170b89947d28021deb05

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

comment:14 Changed 3 years ago by tscrim

  • Keywords days94 added
  • Milestone changed from sage-8.2 to sage-8.3
  • Reviewers changed from Ralf Stephan to Ralf Stephan, Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:15 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

see patchbot

comment:16 Changed 3 years ago by tscrim

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.

comment:17 Changed 3 years ago by git

  • Commit changed from 699c0a2215c0b2472a70170b89947d28021deb05 to cb24f9a42dbddb869f40479b513e21c52d1200ae

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

cb24f9a24883: improve usage of ambigous symbol in doctest

comment:18 Changed 3 years ago by rws

  • Status changed from needs_work to needs_review

comment:19 Changed 3 years ago by vdelecroix

  • Milestone changed from sage-8.3 to sage-8.4

update milestone 8.3 -> 8.4

comment:20 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

Forgot about this. LGTM.

comment:21 follow-up: Changed 3 years ago by git

  • Commit changed from cb24f9a42dbddb869f40479b513e21c52d1200ae to 4d0c51eab2f477499b6f7c64b5e7d179f8063cf2
  • Status changed from positive_review to needs_review

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

4d0c51eMerge with sage-8.3

comment:22 in reply to: ↑ 21 Changed 3 years ago by gh-bryangingechen

  • Status changed from needs_review to positive_review

Replying to 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.

comment:23 Changed 3 years ago by vbraun

  • Branch changed from public/24883 to 4d0c51eab2f477499b6f7c64b5e7d179f8063cf2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.