Opened 11 years ago
Closed 9 years ago
#11726 closed defect (fixed)
Implement univariate Laurent polynomial ring & elements
Reported by:  Kelly Boothby  Owned by:  Kelly Boothby 

Priority:  minor  Milestone:  sage6.2 
Component:  commutative algebra  Keywords:  sd32, Laurent polynomials 
Cc:  Andrew Mathas, Ralf Stephan  Merged in:  
Authors:  Tom Boothby  Reviewers:  Travis Scrimshaw, Ralf Stephan, Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  e27a420 (Commits, GitHub, GitLab)  Commit:  e27a420b0684591d56e5b9eee6c56a3c1c543fed 
Dependencies:  #14261 #15345 #15450 #15843  Stopgaps: 
Description (last modified by )
At present, the implementation of univariate Laurent polynomials is (selfadmittedly) in a sorry state:
############################################################ # This should later get moved to an actual single variate # # implementation with valuation tracking, # # but I don't want to right now. # ############################################################ # We need to come up with a name for the inverse that is easy to search # for in a string *and* doesn't overlap with the name that we already have. # For now, I'm going to use a name mangling with checking method.
This should be fixed.
Attachments (2)
Change History (35)
comment:1 Changed 11 years ago by
Description:  modified (diff) 

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

Summary:  Failure to coerce 1/q into its own LaurentPolynomialRing → Implement univariate Laurent polynomial ring & elements 
comment:3 Changed 11 years ago by
Keywords:  days32 added 

Owner:  changed from Martin Albrecht to Kelly Boothby 
comment:4 Changed 11 years ago by
Keywords:  sd32 added; days32 removed 

Changed 11 years ago by
Attachment:  trac_11726.patch added 

comment:5 Changed 11 years ago by
Status:  new → needs_review 

comment:6 Changed 11 years ago by
Authors:  → Tom Boothby 

comment:7 Changed 11 years ago by
Reviewers:  → PatchBot 

Status:  needs_review → needs_work 
comment:8 Changed 10 years ago by
Cc:  Andrew Mathas added 

comment:9 Changed 9 years ago by
Milestone:  sage5.11 → sage5.12 

Changed 9 years ago by
Attachment:  trac_11726univariate_Laurent_polynomialsts.patch added 

comment:10 Changed 9 years ago by
Dependencies:  → #14261 

Description:  modified (diff) 
Reviewers:  PatchBot → Travis Scrimshaw 
Here's a new version of the patch which applies on sage5.12.beta5 and depends on #14261 (which is currently being reviewed). This also fixes many things that annoy me with when working with Laurent polynomials, it particular division and simplification in the fraction field.
For patchbot:
Apply: trac_11726univariate_Laurent_polynomialsts.patch
comment:11 Changed 9 years ago by
Status:  needs_work → needs_review 

comment:12 Changed 9 years ago by
Milestone:  sage6.1 → sage6.2 

comment:13 Changed 9 years ago by
What is the state of this? How much of this has been done in #15345?
comment:14 Changed 9 years ago by
Branch:  → public/rings/univariate_laurent11726 

Commit:  → 72ae6877d3f4cddd9fa766df414665a6ef8e2ee0 
Dependencies:  #14261 → #14261 #15345 
I've converted the patch to a git branch and made some more fixes to the coercions and doctests. Not much of this was done in #15345, but there are some changes there I merged in to avoid conflicts.
Last 10 new commits:
3c151ec  trac #15345 corrected code

62bf2e9  trac #15345 better coercion

6d4916c  trac #15345 documentation added

9cf030d  Use fraction field of polynomial ring for fraction field of Laurent polynomial ring

00393f3  trac #15345 corrected doctests in hall algebra

f2e9d65  Fixed documentation for hall_algebra.py.

8d54105  Renamed to_fraction to _fraction_pair.

3530943  Merge branch 'u/tscrim/ticket/15345' of trac.sagemath.org:sage into public/rings/univariate_laurent11726

92292dc  Fixes to doctest in Hall algebra and Laurent polynomial ring.

72ae687  More fixes to doctests, coercion, and division.

comment:15 Changed 9 years ago by
Keywords:  Laurent polynomials added 

comment:16 Changed 9 years ago by
Dependencies:  #14261 #15345 → #14261 #15345 #15450 #15843 

Some extra methods for Laurent polynomials in these tickets.
comment:17 Changed 9 years ago by
Branch:  public/rings/univariate_laurent11726 → public/ticket/11726 

Commit:  72ae6877d3f4cddd9fa766df414665a6ef8e2ee0 → a4e01907300ec292c81f3a0ca9e89c921c823522 
I have rebased on 6.2.beta4. I have changed the branch here. Maybe I should have merged ?
New commits:
0186644  #11726: Implement univariate Laurent polynomial rings

a25c37f  Fixed doctests in Hecke algebras and KL polys.

035e9f0  Fixes to doctest in Hall algebra and Laurent polynomial ring.

b462b29  More fixes to doctests, coercion, and division.

a4e0190  trac #11726 fixed one failing doctest

comment:18 Changed 9 years ago by
The new branch is fine (although I prefer the merge because I don't see any harm in merge commits).
comment:19 Changed 9 years ago by
Cc:  Ralf Stephan added 

comment:20 Changed 9 years ago by
Reviewers:  Travis Scrimshaw → Travis Scrimshaw, Ralf Stephan 

Docs build. Long tests are fine in rings/. I would give positive but I'm not sure if you are finished yet.
comment:21 Changed 9 years ago by
Status:  needs_review → positive_review 

Nope, I'm finished with this.
comment:22 Changed 9 years ago by
Status:  positive_review → needs_work 

Fails doctests
sage t src/doc/en/thematic_tutorials/lie/kazhdan_lusztig_polynomials.rst ********************************************************************** File "src/doc/en/thematic_tutorials/lie/kazhdan_lusztig_polynomials.rst", line 17, in doc.en.thematic_tutorials.lie.kazhdan_lusztig_polynomials Failed example: KL.R(s2, s2*s1*s3*s2) Expected: q^3  3*q^2 + 3*q  1 Got: 1 + 3*q  3*q^2 + q^3
comment:23 Changed 9 years ago by
Commit:  a4e01907300ec292c81f3a0ca9e89c921c823522 → f2e6a35346244851dc1b74296a36bb24afd738fb 

Branch pushed to git repo; I updated commit sha1. New commits:
f2e6a35  trac #11726 fix one doctest in the en tutorial for kazhdan lusztig polys

comment:26 Changed 9 years ago by
Commit:  f2e6a35346244851dc1b74296a36bb24afd738fb → 1bac8e5889626702c601613402d6b3af7421585f 

Branch pushed to git repo; I updated commit sha1. New commits:
1bac8e5  trac #11726 correct the other doctest in tutorial

comment:27 Changed 9 years ago by
Status:  needs_review → positive_review 

Confirm all tutorial tests pass. Oh well, good catch...
comment:28 Changed 9 years ago by
Status:  positive_review → needs_work 

sage t long src/sage/algebras/hall_algebra.py # 13 doctests failed sage t long src/sage/groups/braid.py # 8 doctests failed
comment:29 Changed 9 years ago by
Commit:  1bac8e5889626702c601613402d6b3af7421585f → eb091606a87eb23e0961d85a7f87b330ee2d6c85 

comment:30 Changed 9 years ago by
Commit:  eb091606a87eb23e0961d85a7f87b330ee2d6c85 → e27a420b0684591d56e5b9eee6c56a3c1c543fed 

Branch pushed to git repo; I updated commit sha1. New commits:
e27a420  trac #11726 one failing doctest corrected

comment:31 Changed 9 years ago by
Status:  needs_work → needs_review 

I have implemented a __floordiv__
method for Laurent polynomials in one variable, and corrected the doctests. Needs review.
comment:32 Changed 9 years ago by
Reviewers:  Travis Scrimshaw, Ralf Stephan → Travis Scrimshaw, Ralf Stephan, Frédéric Chapoton 

Status:  needs_review → positive_review 
You seem to have this uncanny ability to do things the night (well for me at least) before I'm going to do them.
comment:33 Changed 9 years ago by
Branch:  public/ticket/11726 → e27a420b0684591d56e5b9eee6c56a3c1c543fed 

Resolution:  → fixed 
Status:  positive_review → closed 
This doesn't pass doctests on any recent version of Sage (see patchbot logs)