Opened 7 years ago

Closed 7 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:

Status badges

Description (last modified by nathanwilliams)

The following code breaks: 

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

Change History (12)

comment:1 Changed 7 years ago by nathanwilliams

  • Authors set to Nathan Williams
  • Cc VivianePons stumpc5 added
  • Component changed from PLEASE CHANGE to combinatorics
  • Description modified (diff)
  • Keywords sagedays64.5 coxeter descent added
  • Type changed from PLEASE CHANGE to defect

comment:2 Changed 7 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 7 years ago by nathanwilliams

  • Branch set to u/nathanwilliams/bug__circular_descent_check_in_weylgroups

comment:4 Changed 7 years ago by nathanwilliams

  • Commit set to 8fe1bb853913ea62a065bf5b53fa460928354815
  • Status changed from new to needs_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 7 years ago by chapoton

see #15456

comment:6 Changed 7 years ago by nathanwilliams

Oy. Veni, vidi, reliqui.

comment:7 follow-up: Changed 7 years ago by chapoton

  • Status changed from needs_review to needs_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 7 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 7 years ago by git

  • Commit changed from 8fe1bb853913ea62a065bf5b53fa460928354815 to ab4999f27f1e84c2659c544ccc1b5ee3afe7b729

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

​ab4999fmore intelligent fix a la stump

comment:10 Changed 7 years ago by nathanwilliams

  • Status changed from needs_work to needs_review

comment:11 Changed 7 years ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

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

comment:12 Changed 7 years ago by vbraun

  • Branch changed from u/nathanwilliams/bug__circular_descent_check_in_weylgroups to ab4999f27f1e84c2659c544ccc1b5ee3afe7b729
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.