Opened 8 years ago
Closed 6 years ago
#11726 closed defect (fixed)
Implement univariate Laurent polynomial ring & elements
Reported by:  boothby  Owned by:  boothby 

Priority:  minor  Milestone:  sage6.2 
Component:  commutative algebra  Keywords:  sd32, Laurent polynomials 
Cc:  andrew.mathas, rws  Merged in:  
Authors:  Tom Boothby  Reviewers:  Travis Scrimshaw, Ralf Stephan, Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  e27a420 (Commits)  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 8 years ago by
 Description modified (diff)
comment:2 Changed 8 years ago by
 Description modified (diff)
 Summary changed from Failure to coerce 1/q into its own LaurentPolynomialRing to Implement univariate Laurent polynomial ring & elements
comment:3 Changed 8 years ago by
 Keywords days32 added
 Owner changed from malb to boothby
comment:4 Changed 8 years ago by
 Keywords sd32 added; days32 removed
Changed 8 years ago by
comment:5 Changed 8 years ago by
 Status changed from new to needs_review
comment:6 Changed 8 years ago by
comment:7 Changed 8 years ago by
 Reviewers set to PatchBot
 Status changed from needs_review to needs_work
comment:8 Changed 7 years ago by
 Cc andrew.mathas added
comment:9 Changed 6 years ago by
 Milestone changed from sage5.11 to sage5.12
Changed 6 years ago by
comment:10 Changed 6 years ago by
 Dependencies set to #14261
 Description modified (diff)
 Reviewers changed from PatchBot to 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 6 years ago by
 Status changed from needs_work to needs_review
comment:12 Changed 6 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:13 Changed 6 years ago by
What is the state of this? How much of this has been done in #15345?
comment:14 Changed 6 years ago by
 Branch set to public/rings/univariate_laurent11726
 Commit set to 72ae6877d3f4cddd9fa766df414665a6ef8e2ee0
 Dependencies changed from #14261 to #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 6 years ago by
 Keywords Laurent polynomials added
comment:16 Changed 6 years ago by
 Dependencies changed from #14261 #15345 to #14261 #15345 #15450 #15843
Some extra methods for Laurent polynomials in these tickets.
comment:17 Changed 6 years ago by
 Branch changed from public/rings/univariate_laurent11726 to public/ticket/11726
 Commit changed from 72ae6877d3f4cddd9fa766df414665a6ef8e2ee0 to 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 6 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 6 years ago by
 Cc rws added
comment:20 Changed 6 years ago by
 Reviewers changed from Travis Scrimshaw to 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 6 years ago by
 Status changed from needs_review to positive_review
Nope, I'm finished with this.
comment:22 Changed 6 years ago by
 Status changed from positive_review to 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 6 years ago by
 Commit changed from a4e01907300ec292c81f3a0ca9e89c921c823522 to 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:24 Changed 6 years ago by
 Status changed from needs_work to needs_review
I have corrected the doctest.
comment:25 Changed 6 years ago by
I think there was more than one, please test.
comment:26 Changed 6 years ago by
 Commit changed from f2e6a35346244851dc1b74296a36bb24afd738fb to 1bac8e5889626702c601613402d6b3af7421585f
Branch pushed to git repo; I updated commit sha1. New commits:
1bac8e5  trac #11726 correct the other doctest in tutorial

comment:27 Changed 6 years ago by
 Status changed from needs_review to positive_review
Confirm all tutorial tests pass. Oh well, good catch...
comment:28 Changed 6 years ago by
 Status changed from positive_review to 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 6 years ago by
 Commit changed from 1bac8e5889626702c601613402d6b3af7421585f to eb091606a87eb23e0961d85a7f87b330ee2d6c85
comment:30 Changed 6 years ago by
 Commit changed from eb091606a87eb23e0961d85a7f87b330ee2d6c85 to e27a420b0684591d56e5b9eee6c56a3c1c543fed
Branch pushed to git repo; I updated commit sha1. New commits:
e27a420  trac #11726 one failing doctest corrected

comment:31 Changed 6 years ago by
 Status changed from needs_work to needs_review
I have implemented a __floordiv__
method for Laurent polynomials in one variable, and corrected the doctests. Needs review.
comment:32 Changed 6 years ago by
 Reviewers changed from Travis Scrimshaw, Ralf Stephan to Travis Scrimshaw, Ralf Stephan, Frédéric Chapoton
 Status changed from needs_review to 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 6 years ago by
 Branch changed from public/ticket/11726 to e27a420b0684591d56e5b9eee6c56a3c1c543fed
 Resolution set to fixed
 Status changed from positive_review to closed
This doesn't pass doctests on any recent version of Sage (see patchbot logs)