Opened 3 years ago

Closed 2 weeks ago

#11726 closed defect (fixed)

Implement univariate Laurent polynomial ring & elements

Reported by: boothby Owned by: boothby
Priority: minor Milestone: sage-6.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 tscrim)

At present, the implementation of univariate Laurent polynomials is (self-admittedly) 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.

Apply: trac_11726-univariate_Laurent_polynomials-ts.patch

Attachments (2)

trac_11726.patch (45.9 KB) - added by boothby 3 years ago.
trac_11726-univariate_Laurent_polynomials-ts.patch (88.5 KB) - added by tscrim 7 months ago.

Download all attachments as: .zip

Change History (35)

comment:1 Changed 3 years ago by boothby

  • Description modified (diff)

comment:2 Changed 3 years ago by boothby

  • 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 3 years ago by boothby

  • Keywords days32 added
  • Owner changed from malb to boothby

comment:4 Changed 3 years ago by boothby

  • Keywords sd32 added; days32 removed

Changed 3 years ago by boothby

comment:5 Changed 3 years ago by boothby

  • Status changed from new to needs_review

comment:6 Changed 3 years ago by boothby

  • Authors set to Tom Boothby

comment:7 Changed 2 years ago by davidloeffler

  • Reviewers set to PatchBot
  • Status changed from needs_review to needs_work

This doesn't pass doctests on any recent version of Sage (see patchbot logs)

comment:8 Changed 14 months ago by andrew.mathas

  • Cc andrew.mathas added

comment:9 Changed 8 months ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:10 Changed 7 months ago by tscrim

  • 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 sage-5.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_11726-univariate_Laurent_polynomials-ts.patch​

comment:11 Changed 7 months ago by tscrim

  • Status changed from needs_work to needs_review

comment:12 Changed 3 months ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:13 Changed 2 months ago by darij

What is the state of this? How much of this has been done in #15345?

comment:14 Changed 2 months ago by tscrim

  • Branch set to public/rings/univariate_laurent-11726
  • 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:

3c151ectrac #15345 corrected code
62bf2e9trac #15345 better coercion
6d4916ctrac #15345 documentation added
9cf030dUse fraction field of polynomial ring for fraction field of Laurent polynomial ring
00393f3trac #15345 corrected doctests in hall algebra
f2e9d65Fixed documentation for hall_algebra.py.
8d54105Renamed to_fraction to _fraction_pair.
3530943Merge branch 'u/tscrim/ticket/15345' of trac.sagemath.org:sage into public/rings/univariate_laurent-11726
92292dcFixes to doctest in Hall algebra and Laurent polynomial ring.
72ae687More fixes to doctests, coercion, and division.

comment:15 Changed 7 weeks ago by chapoton

  • Keywords Laurent polynomials added

comment:16 Changed 7 weeks ago by tscrim

  • Dependencies changed from #14261 #15345 to #14261 #15345 #15450 #15843

Some extra methods for Laurent polynomials in these tickets.

comment:17 Changed 6 weeks ago by chapoton

  • Branch changed from public/rings/univariate_laurent-11726 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
a25c37fFixed doctests in Hecke algebras and KL polys.
035e9f0Fixes to doctest in Hall algebra and Laurent polynomial ring.
b462b29More fixes to doctests, coercion, and division.
a4e0190trac #11726 fixed one failing doctest

comment:18 Changed 6 weeks ago by tscrim

The new branch is fine (although I prefer the merge because I don't see any harm in merge commits).

comment:19 Changed 6 weeks ago by rws

  • Cc rws added

comment:20 Changed 3 weeks ago by rws

  • 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 3 weeks ago by tscrim

  • Status changed from needs_review to positive_review

Nope, I'm finished with this.

comment:22 Changed 3 weeks ago by vbraun

  • 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 3 weeks ago by git

  • Commit changed from a4e01907300ec292c81f3a0ca9e89c921c823522 to f2e6a35346244851dc1b74296a36bb24afd738fb

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

f2e6a35trac #11726 fix one doctest in the en tutorial for kazhdan lusztig polys

comment:24 Changed 3 weeks ago by chapoton

  • Status changed from needs_work to needs_review

I have corrected the doctest.

comment:25 Changed 3 weeks ago by vbraun

I think there was more than one, please test.

comment:26 Changed 3 weeks ago by git

  • Commit changed from f2e6a35346244851dc1b74296a36bb24afd738fb to 1bac8e5889626702c601613402d6b3af7421585f

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

1bac8e5trac #11726 correct the other doctest in tutorial

comment:27 Changed 3 weeks ago by rws

  • Status changed from needs_review to positive_review

Confirm all tutorial tests pass. Oh well, good catch...

comment:28 Changed 3 weeks ago by vbraun

  • 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 2 weeks ago by git

  • Commit changed from 1bac8e5889626702c601613402d6b3af7421585f to eb091606a87eb23e0961d85a7f87b330ee2d6c85

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

e07c76eMerge branch 'public/ticket/11726' of trac.sagemath.org:sage into 11726
eb09160trac #11726 implement floordiv for laurent polys in one variable

comment:30 Changed 2 weeks ago by git

  • Commit changed from eb091606a87eb23e0961d85a7f87b330ee2d6c85 to e27a420b0684591d56e5b9eee6c56a3c1c543fed

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

e27a420trac #11726 one failing doctest corrected

comment:31 Changed 2 weeks ago by chapoton

  • 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 2 weeks ago by tscrim

  • 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 2 weeks ago by vbraun

  • Branch changed from public/ticket/11726 to e27a420b0684591d56e5b9eee6c56a3c1c543fed
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.