Opened 8 years ago

Closed 8 years ago

#18610 closed defect (fixed)

Bug: Circular Descent Check in WeylGroups

Reported by: nathanwilliams Owned by:
Priority: major Milestone: sage-6.8
Component: combinatorics Keywords: sagedays64.5, coxeter, descent
Cc: VivianePons, stumpc5 Merged in:
Authors: Nathan Williams Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: ab4999f (Commits, GitHub, GitLab) Commit: ab4999f27f1e84c2659c544ccc1b5ee3afe7b729
Dependencies: Stopgaps:

GitHub link to the corresponding issue

Description (last modified by nathanwilliams)

The following code breaks:

  WeylGroup(['A',2]).long_element().has_left_descent(1)

Change History (12)

comment:1 Changed 8 years ago by nathanwilliams

Authors: Nathan Williams
Cc: VivianePons stumpc5 added
Component: PLEASE CHANGEcombinatorics
Description: modified (diff)
Keywords: sagedays64.5 coxeter descent added
Type: PLEASE CHANGEdefect

comment:2 Changed 8 years ago by tscrim

This is because the Weyl group element implements has_descent, which is the standard entry point, and the default implementations do the following call structure:

has_descent -> has_left_descent <-> has_right_descent

since I think most code calls has_descent and gives the shortest path function call path if someone only implements has_left_descent. So the solution would be to explicitly have has_left_descent(i) call has_descent(i, side='left').

comment:3 Changed 8 years ago by nathanwilliams

Branch: u/nathanwilliams/bug__circular_descent_check_in_weylgroups

comment:4 Changed 8 years ago by nathanwilliams

Commit: 8fe1bb853913ea62a065bf5b53fa460928354815
Status: newneeds_review

The suggestion is essentially a good one, but fails because CoxeterGroups().example() is in a category where this again will cause an infinite loop. Changing has_right_descent(i) in the way suggested works.


New commits:

5716df5Fixed according to Travis's suggestions
4404133Fixed according to Travis's suggestions, and now actually running I think
8fe1bb8Tab fixed

comment:5 Changed 8 years ago by chapoton

see #15456

comment:6 Changed 8 years ago by nathanwilliams

Oy. Veni, vidi, reliqui.

comment:7 Changed 8 years ago by chapoton

Status: needs_reviewneeds_work

I think this really needs to be solved.

Either you go this way, but add a doctest, or you use the other ticket.

comment:8 in reply to:  7 Changed 8 years ago by nathanwilliams

Replying to chapoton:

I think this really needs to be solved.

Either you go this way, but add a doctest, or you use the other ticket.

I had added a test, so could I ask you to be more specific about what you would like?

comment:9 Changed 8 years ago by git

Commit: 8fe1bb853913ea62a065bf5b53fa460928354815ab4999f27f1e84c2659c544ccc1b5ee3afe7b729

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

ab4999fmore intelligent fix a la stump

comment:10 Changed 8 years ago by nathanwilliams

Status: needs_workneeds_review

comment:11 Changed 8 years ago by chapoton

Reviewers: Frédéric Chapoton
Status: needs_reviewpositive_review

ok, this does not seem to break anything. Let it be

comment:12 Changed 8 years ago by vbraun

Branch: u/nathanwilliams/bug__circular_descent_check_in_weylgroupsab4999f27f1e84c2659c544ccc1b5ee3afe7b729
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.