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

Branching rule A2->A1 never returns #15023

Closed
vbraun opened this issue Aug 8, 2013 · 16 comments
Closed

Branching rule A2->A1 never returns #15023

vbraun opened this issue Aug 8, 2013 · 16 comments

Comments

@vbraun
Copy link
Member

vbraun commented Aug 8, 2013

The following goes into an infinite loop and never returns:

sage: A2 = WeylCharacterRing('A2', style='coroots')
sage: A1 = WeylCharacterRing('A1', style='coroots')
sage: A2(1,0).branch(A1)

Apply:

CC: @sagetrac-sage-combinat @dwbump

Component: combinatorics

Keywords: "branching rule"

Author: Daniel Bump

Reviewer: Travis Scrimshaw

Merged: sage-5.12.beta3

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

@vbraun vbraun added this to the sage-5.12 milestone Aug 8, 2013
@dwbump
Copy link
Mannequin

dwbump mannequin commented Aug 9, 2013

comment:1

This works:

sage: A2(1,0).branch(A1,rule="levi")
A1(0) + A1(1)

But it remains a problem that the code is not very forgiving if you forget to specify a branching rule. There are two possible approaches to fixing this.

The first would be to raise a ValueError if the branching rule is omitted. The second would be to try to guess the branching rule by consulting a table. The first way would be easy to implement and I can submit a patch for it.

@vbraun
Copy link
Member Author

vbraun commented Aug 9, 2013

comment:2

Though generally the default rule works, its just here that it doesn't. It is a convenient feature to let Sage guess the branching rule, so having some heuristic / a table would be nice.

If you really don't want to have a default then rule shouldn't be a optional keyword argument: def branch(weyl_character_ring, rule): would always force the user to enter something.

@dwbump
Copy link
Mannequin

dwbump mannequin commented Aug 12, 2013

comment:3

Though generally the default rule works, its just here that it doesn't. It is a
convenient feature to let Sage guess the branching rule, so having some heuristic / a table would be > nice.

I agree and I am thinking the default branching rule should be to do the obvious thing
in the following cases:

(1) Top Cartan type is irreducible and restriction is to a Levi.
(2) The top group is type A_r, and the bottom group is embedded
    by means of its standard representation. This includes three
    cases B_r -> A_{2r}, C_r -> A_{2r-1} and D_r -> A_{2r-1}.
    Equivalently, either O(n) -> SL(n) or Sp(2n) -> SL(2n).
(3) O(n) -> O(n-1).

That might be enough for an initial implementation. I thought of adding Sp(2n)->Sp(2n-2) to this list but this BR factors through Sp(2n-2)xA1 which is a Levi branching.

These are the default rules implemented by the patch. If a default rule is not found, an error is raised.

@dwbump dwbump mannequin self-assigned this Aug 12, 2013
@dwbump
Copy link
Mannequin

dwbump mannequin commented Aug 13, 2013

Attachment: trac_15023_branch.patch.gz

#15023: implements correct default branching rules

@dwbump dwbump mannequin added the s: needs review label Aug 13, 2013
@dwbump
Copy link
Mannequin

dwbump mannequin commented Aug 13, 2013

Author: bump

@dwbump
Copy link
Mannequin

dwbump mannequin commented Aug 13, 2013

Changed keywords from none to "branching rule"

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Aug 13, 2013

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 13, 2013

Changed author from bump to Dan Bump

@tscrim
Copy link
Collaborator

tscrim commented Aug 13, 2013

comment:8

Hey Dan,

Here's a review patch which removes the bare except: statements, does some doc formatting (in preparation of #15042), and re-organizes the code to make it easier to read (to me at least). If you're happy with my changes, you can go ahead and set this to positive review.

@dwbump
Copy link
Mannequin

dwbump mannequin commented Aug 13, 2013

comment:9

Thanks Travis. The reviewer patch looks good. I'll wait until tomorrow or so to set pos review since Volker Braun might have some comment.

@vbraun
Copy link
Member Author

vbraun commented Aug 13, 2013

comment:10

Looks good to me, thanks!

@tscrim
Copy link
Collaborator

tscrim commented Aug 13, 2013

comment:11

Your welcome. Thanks.

@tscrim
Copy link
Collaborator

tscrim commented Aug 13, 2013

comment:12

Attachment: trac_15023-review-ts.patch.gz

I had to do a minor tweak to catch an A1 issue (change an elif to an if).

@jdemeyer
Copy link

Merged: sage-5.12.beta3

@fchapoton
Copy link
Contributor

Changed author from Dan Bump to Daniel Bump

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

4 participants