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

Kazhdan-Lusztig polynomials, Bruhat order, and related features #7751

Closed
dwbump mannequin opened this issue Dec 22, 2009 · 20 comments
Closed

Kazhdan-Lusztig polynomials, Bruhat order, and related features #7751

dwbump mannequin opened this issue Dec 22, 2009 · 20 comments

Comments

@dwbump
Copy link
Mannequin

dwbump mannequin commented Dec 22, 2009

This patch includes algorithms for the Bruhat order, Kazhdan-Lusztig polynomials, improvements to the __repr__ method of WeylGroup elements, and other enhancements.

Mike Hansen is working on an interface to coxeter3, which is be able to compute Kazhdan-Lusztig polynomials rather faster. However I think this patch still contains things that are needed.

For discussion see this thread:

http://groups.google.com/group/sage-combinat-devel/browse_thread/thread/d324f6fcb6d2a436?hl=en

This patch depends on #7753 and #7754.

CC: @sagetrac-sage-combinat

Component: combinatorics

Keywords: Kazhdan-Lusztig, Bruhat order

Author: Daniel Bump

Reviewer: David Roe, Brant Jones

Merged: sage-4.3.3.alpha1

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

@dwbump dwbump mannequin added this to the sage-4.3.3 milestone Dec 22, 2009
@dwbump dwbump mannequin added c: algebra labels Dec 22, 2009
@dwbump dwbump mannequin assigned aghitza Dec 22, 2009
@dwbump dwbump mannequin added c: combinatorics and removed c: algebra labels Dec 22, 2009
@dwbump dwbump mannequin assigned dwbump and unassigned aghitza Dec 22, 2009
@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Dec 23, 2009

comment:2

I made a minor change so that the Kazhdan-Lusztig computation doesn't
hang in the affine case. I've only done much testing for finite Weyl groups
but I suppose it is correct in the affine case.

@dwbump

This comment has been minimized.

@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Jan 3, 2010

comment:3

I have revised the patch. It now depends on #7753 and #7754. The revised patch makes use of the Bruhat order as implemented in #7753 and makes the Kazhdan-Lusztig polynomials using @cached_method. Other changes allow the base ring to be a LaurentPolynomialRing.

The patch is much faster now, something like 50% faster.

@dwbump dwbump mannequin added the s: needs review label Jan 3, 2010
@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Jan 23, 2010

comment:5

Rebased to Sage 4.3.1.

@roed314
Copy link
Contributor

roed314 commented Feb 9, 2010

Reviewer: roed

@roed314
Copy link
Contributor

roed314 commented Feb 9, 2010

comment:6

Looks good. Here are a few comments. After these are addressed, I'll be happy to give this a positive review.

  • sage/combinat/kazhdan_lusztig.py

    • typo in your e-mail address.
    • the method of determining KL._base_ring_type seems a little strange. Why not use is_Polynomial and isinstance(q, LaurentPolynomial)?
    • KazhdanLusztigPolynomial should inherit from SageObject. That allows pickling, etc.
  • sage/combinat/root_system/weyl_group.py
    *In WeylGroup_gens, __classcall_ needs another trailing underscore. Include a doctest to make sure that this feature works!

    • Can you include a doctest in WeylGroupElement.__repr__? I know it's tested elsewhere, but...

In general, do you have a reason to use __call__ explicitly, rather than parentheses? Similarly, you don't need to explicitly call repr: using %s in a string will do that for you automatically.

@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Feb 11, 2010

comment:7

I addressed most of David Roe's comments.

But the email address is not a typo.

Brant Jones is also looking at the patch.

@dwbump dwbump mannequin added s: needs review and removed s: needs work labels Feb 11, 2010
@sagetrac-brant-c-jones
Copy link
Mannequin

sagetrac-brant-c-jones mannequin commented Feb 11, 2010

comment:8

Patch review: trac_7751

This patch implements Kazhdan--Lusztig polynomials and R-polynomials associated to pairs of Weyl group elements in arbitrary type using standard recursive formulas. I have verified the results of this code against the Kazhdan--Lusztig polynomials produced by GAP/Chevie for all pairs of elements in finite type A_4. I also verified this code against GAP/Chevie for all pairs in affine type A_2 (having 3 generators) for all pairs of elements with Coxeter length less than or equal to 5. This patch correctly implements useful mathematics and the documentation includes references to relevant mathematical literature.

-- Brant Jones

@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Feb 11, 2010

Changed reviewer from roed to roed, Brant Jones

@dwbump dwbump mannequin changed the title Kazhdan-Lusztig polynomials, Bruhat order, and related features [with patch, needs review] Kazhdan-Lusztig polynomials, Bruhat order, and related features Feb 11, 2010
@roed314
Copy link
Contributor

roed314 commented Feb 11, 2010

comment:11

I agree. Positive review.

@roed314
Copy link
Contributor

roed314 commented Feb 11, 2010

Author: bump

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 11, 2010

comment:13

Applying the patch to this hierarchy (minus #8232), I get

patching file sage/combinat/root_system/weyl_group.py
Hunk #5 FAILED at 145
1 out of 13 hunks FAILED -- saving rejects to file sage/combinat/root_system/weyl_group.py.rej

The reject:

--- weyl_group.py
+++ weyl_group.py
@@ -138,6 +146,7 @@
         self.n = lattice.dimension() # Really needed?
         # MatrixGroup_gens takes plain matrices as input. So we can't do:
         #MatrixGroup_gens.__init__(self, list(self.simple_reflections()))
+        self._prefix = prefix
         MatrixGroup_gens.__init__(self, [self.morphism_matrix(self.lattice().si
mple_reflection(i)) for i in self.index_set()])
 
     @cached_method

@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Feb 12, 2010

comment:14

If this conflict occurs, you may resolve just making sure the line
self._prefix = prefix occurs somewhere in the __init__ method of
WeylGroup.

@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Feb 12, 2010

comment:15

Correction: the line self._prefix = prefix should be somewhere in the init method of WeylGroup_gens
(not WeylGroup).

@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Feb 13, 2010

Kazhdan-Lusztig polynomials, Bruhat order, improved __repr__ for WeylGroups.

@dwbump
Copy link
Mannequin Author

dwbump mannequin commented Feb 13, 2010

comment:16

Attachment: kazhdan_lusztig.patch.gz

Patch rebased to sage-4.3.3.alpha0. This fixes the conflict
reported by mpatel with no other changes.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Feb 14, 2010

Changed author from bump to Daniel Bump

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Feb 14, 2010

comment:17

Daniel, I have committed the attachment kazhdan_lusztig.patch in your name, since the patch doesn't contain your full name.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Feb 14, 2010

Merged: sage-4.3.3.alpha1

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Feb 14, 2010

Changed reviewer from roed, Brant Jones to David Roe, Brant Jones

@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Feb 14, 2010
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

2 participants