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

Highly confusing behavior of RootSystem related objects (lattices and ambient space) #15325

Open
sagetrac-davidamadore mannequin opened this issue Oct 25, 2013 · 1 comment

Comments

@sagetrac-davidamadore
Copy link
Mannequin

sagetrac-davidamadore mannequin commented Oct 25, 2013

I'm not sure to what extent the following behavior is deliberate or simply unavoidable, so maybe this bug report should be reclassified as bad documentation. Also, I'm not sure whether I should be filing several different bug reports (but I do believe the issues I raise are very closely related).

My main issue is that for mathematicians who are used to thinking of the weight lattice of a root system living inside of the root lattice, itself living inside some common euclidean ambient space (which, I think, is standard practice), Sage's behavior is mind-boggingly confusing, if not outright insane.

Exhibit A (shows that elements of the root lattice and its ambient space, while comparing for equality, behave very differently when it comes to scalar product):

sage: rt = RootSystem(['E',8])
sage: lat = rt.root_lattice()
sage: spc = rt.ambient_space()
sage: lat.simple_root(1) == spc.simple_root(1)
True
sage: lat.simple_coroot(2) == spc.simple_coroot(2)
True
sage: lat.simple_root(1).scalar(lat.simple_coroot(2))
0
sage: spc.simple_root(1).scalar(spc.simple_coroot(2))
0
sage: spc.simple_root(1).scalar(lat.simple_coroot(2))
-1/2
sage: lat.simple_root(1).scalar(spc.simple_coroot(2))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-9-61492d33e12b> in <module>()
----> 1 lat.simple_root(Integer(1)).scalar(spc.simple_coroot(Integer(2)))

/work/sage-5.12/local/lib/python2.7/site-packages/sage/combinat/root_system/root_space.pyc in scalar(self, lambdacheck)
    245         # Find some better test
    246         if not (lambdacheck in self.parent().coroot_lattice() or lambdacheck in self.parent().coroot_space()):
--> 247             raise TypeError, "%s is not in a coroot lattice/space"%(lambdacheck)
    248         zero = self.parent().base_ring().zero()
    249         cartan_matrix = self.parent().dynkin_diagram()

TypeError: (1, 1, 0, 0, 0, 0, 0, 0) is not in a coroot lattice/space

Exhibit B (shows that Sage does not consider the root lattice as living inside the weight lattice):

sage: rt = RootSystem(['E',8])
sage: wlat = rt.weight_lattice()
sage: lat = rt.root_lattice()
sage: wlat.simple_root(1) == lat.simple_root(1)
True
sage: lat.simple_root(1) in lat
True
sage: wlat.simple_root(1) in lat
False

Exhibit C (shows how confusing the embedding of the weight lattice in the ambient space is):

sage: rt = RootSystem(['A',2])
sage: wlat = rt.weight_lattice()
sage: lat = rt.root_lattice()
sage: spc = rt.ambient_space()
sage: lat.simple_root(2) == wlat.simple_root(2)
True
sage: spc.simple_root(2)
(0, 1, -1)
sage: spc(lat.simple_root(2))
(0, 1, -1)
sage: spc(wlat.simple_root(2))
(1, 2, 0)

I'm not sure how all of this should be fixed, but certainly when x==y returns True then x and y should have a very similar behavior, and in particular x in y.parent() should also return True.

CC: @sagetrac-sage-combinat @nthiery

Component: combinatorics

Keywords: root-system

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

@sagetrac-davidamadore sagetrac-davidamadore mannequin added this to the sage-6.1 milestone Oct 25, 2013
@tscrim
Copy link
Collaborator

tscrim commented Nov 3, 2013

comment:1

Nicolas can probably do a better job at explaining the math design choices, but hopefully this won't be too far off the mark.

For C, this is "correct" and is due to the projection used, i.e. (1,1,1) = (0,0,0). I am slightly of the opinion that we need to standardize input when doing things like scalar products (maybe making the last coordinate 0, especially for equality checking). Some things are just easier in a larger space (such as the definition on a root).

For A, my knowledge of E8 is somewhat limited. I'd expect the TypeError since there is no coercion occurring (perhaps there should be), so it's trying to do things in the base object. Here that's the root lattice. I do think that the scalar should be consistent between the root lattice and the ambient space.

For B, this does not surprise me (too much), again because it's trying to do things in the root lattice and that is a sublattice of the weight lattice. In general, I don't think one cannot take something from the weight lattice and express it in the root lattice.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
skipgaribaldi added a commit to skipgaribaldi/sage that referenced this issue Sep 26, 2024
skipgaribaldi added a commit to skipgaribaldi/sage that referenced this issue Sep 26, 2024
skipgaribaldi added a commit to skipgaribaldi/sage that referenced this issue Oct 1, 2024
skipgaribaldi added a commit to skipgaribaldi/sage that referenced this issue Oct 1, 2024
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 5, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Goal of this PR:
   * Fix Exhibit A in sagemath#15325
   * Exhibits B and C in the issue are ignored because they are not
strictly speaking errors

Status quo:
   * Some inner products for elements of root spaces or ambient spaces
give wrong answers or raise exceptions

Fix:
   * Adds a few lines of code to scalar product functions in
ambient_space and root_space
   * TESTS were added to scalar product member functions to exhibit
fixing this issue

Notes:
   * I am unable to verify that the documentation builds because I
cannot build the documentation (build problems with this version of
Sage)

A more thorough test of this functionality, which checks Exhibit A from
the issue on more cases, is provided by the following code snippet:

```
for ct in [['G', 2], ['F', 4], ['E', 8], ['E', 7]] + [[x, 6] for x in
['A', 'B', 'C', 'D', 'E']]:
    rt = RootSystem(ct)
    print(ct)
    lat = rt.root_lattice()
    spc = rt.ambient_space()
    ell = lat.rank()
    C = rt.cartan_matrix()
    for i in range(1,ell+1):
        assert lat.simple_root(i) == spc.simple_root(i)
        assert lat.simple_coroot(i) == spc.simple_coroot(i)
        for j in range(1,ell+1):
            aprod = C[j-1,i-1]
            assert lat.simple_root(i).scalar(lat.simple_coroot(j)) ==
aprod
            assert spc.simple_root(i).scalar(spc.simple_coroot(j)) ==
aprod
            assert spc.simple_root(i).scalar(lat.simple_coroot(j)) ==
aprod
            assert lat.simple_root(i).scalar(spc.simple_coroot(j)) ==
aprod

```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [X] The title is concise and informative.
- [X] The description explains in detail what this PR is about.
- [X] I have linked a relevant issue or discussion.
- [X] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38750
Reported by: Skip G
Reviewer(s): Skip G, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 9, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Goal of this PR:
   * Fix Exhibit A in sagemath#15325
   * Exhibits B and C in the issue are ignored because they are not
strictly speaking errors

Status quo:
   * Some inner products for elements of root spaces or ambient spaces
give wrong answers or raise exceptions

Fix:
   * Adds a few lines of code to scalar product functions in
ambient_space and root_space
   * TESTS were added to scalar product member functions to exhibit
fixing this issue

Notes:
   * I am unable to verify that the documentation builds because I
cannot build the documentation (build problems with this version of
Sage)

A more thorough test of this functionality, which checks Exhibit A from
the issue on more cases, is provided by the following code snippet:

```
for ct in [['G', 2], ['F', 4], ['E', 8], ['E', 7]] + [[x, 6] for x in
['A', 'B', 'C', 'D', 'E']]:
    rt = RootSystem(ct)
    print(ct)
    lat = rt.root_lattice()
    spc = rt.ambient_space()
    ell = lat.rank()
    C = rt.cartan_matrix()
    for i in range(1,ell+1):
        assert lat.simple_root(i) == spc.simple_root(i)
        assert lat.simple_coroot(i) == spc.simple_coroot(i)
        for j in range(1,ell+1):
            aprod = C[j-1,i-1]
            assert lat.simple_root(i).scalar(lat.simple_coroot(j)) ==
aprod
            assert spc.simple_root(i).scalar(spc.simple_coroot(j)) ==
aprod
            assert spc.simple_root(i).scalar(lat.simple_coroot(j)) ==
aprod
            assert lat.simple_root(i).scalar(spc.simple_coroot(j)) ==
aprod

```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [X] The title is concise and informative.
- [X] The description explains in detail what this PR is about.
- [X] I have linked a relevant issue or discussion.
- [X] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38750
Reported by: Skip G
Reviewer(s): Skip G, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 11, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Goal of this PR:
   * Fix Exhibit A in sagemath#15325
   * Exhibits B and C in the issue are ignored because they are not
strictly speaking errors

Status quo:
   * Some inner products for elements of root spaces or ambient spaces
give wrong answers or raise exceptions

Fix:
   * Adds a few lines of code to scalar product functions in
ambient_space and root_space
   * TESTS were added to scalar product member functions to exhibit
fixing this issue

Notes:
   * I am unable to verify that the documentation builds because I
cannot build the documentation (build problems with this version of
Sage)

A more thorough test of this functionality, which checks Exhibit A from
the issue on more cases, is provided by the following code snippet:

```
for ct in [['G', 2], ['F', 4], ['E', 8], ['E', 7]] + [[x, 6] for x in
['A', 'B', 'C', 'D', 'E']]:
    rt = RootSystem(ct)
    print(ct)
    lat = rt.root_lattice()
    spc = rt.ambient_space()
    ell = lat.rank()
    C = rt.cartan_matrix()
    for i in range(1,ell+1):
        assert lat.simple_root(i) == spc.simple_root(i)
        assert lat.simple_coroot(i) == spc.simple_coroot(i)
        for j in range(1,ell+1):
            aprod = C[j-1,i-1]
            assert lat.simple_root(i).scalar(lat.simple_coroot(j)) ==
aprod
            assert spc.simple_root(i).scalar(spc.simple_coroot(j)) ==
aprod
            assert spc.simple_root(i).scalar(lat.simple_coroot(j)) ==
aprod
            assert lat.simple_root(i).scalar(spc.simple_coroot(j)) ==
aprod

```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [X] The title is concise and informative.
- [X] The description explains in detail what this PR is about.
- [X] I have linked a relevant issue or discussion.
- [X] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38750
Reported by: Skip G
Reviewer(s): Skip G, Travis Scrimshaw
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