Skip to content

Commit

Permalink
gh-38750: Corrects some inner products in root systems
Browse files Browse the repository at this point in the history
    
<!-- ^ 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 #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 #12345". -->

Goal of this PR:
   * Fix Exhibit A in #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,
-->
<!-- - #12345: short description why this is a dependency -->
<!-- - #34567: ... -->
    
URL: #38750
Reported by: Skip G
Reviewer(s): Skip G, Travis Scrimshaw
  • Loading branch information
Release Manager committed Oct 11, 2024
2 parents 128927e + e08bb52 commit 0ac3318
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 11 deletions.
26 changes: 21 additions & 5 deletions src/sage/combinat/root_system/ambient_space.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,7 @@ def _repr_(self):

def inner_product(self, lambdacheck):
"""
The scalar product with elements of the coroot lattice
embedded in the ambient space.
The scalar product with elements of the ambient space.
EXAMPLES::
Expand All @@ -379,15 +378,32 @@ def inner_product(self, lambdacheck):
(-1, 0, 0)
sage: a.inner_product(a)
2
TESTS:
Verify that :issue:`15325` (A) is fixed::
sage: rt = RootSystem(['E', 8])
sage: lat = rt.root_lattice()
sage: spc = rt.ambient_space()
sage: spc.simple_root(1).scalar(lat.simple_coroot(2))
0
"""
if self.parent() is not lambdacheck.parent():
try:
# see if lambdacheck can be converted to the ambient space
lambdacheck = self.parent()(lambdacheck)
except (TypeError, ValueError):
raise TypeError(f"unable to coerce {lambdacheck} into {self.parent()}")

# self and lambdacheck both belong to the same ambient space, so use the inner product there
self_mc = self._monomial_coefficients
lambdacheck_mc = lambdacheck._monomial_coefficients

result = self.parent().base_ring().zero()
for t,c in lambdacheck_mc.items():
if t not in self_mc:
continue
result += c*self_mc[t]
if t in self_mc:
result += c*self_mc[t]
return result

scalar = inner_product
Expand Down
33 changes: 27 additions & 6 deletions src/sage/combinat/root_system/root_space.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,34 @@ def scalar(self, lambdacheck):
[-1 2 -1 0]
[ 0 -1 2 -1]
[ 0 0 -2 2]
TESTS:
Verify that :issue:`15325` (A) is fixed::
sage: rt = RootSystem(['E', 8])
sage: lat = rt.root_lattice()
sage: spc = rt.ambient_space()
sage: lat.simple_root(1).scalar(spc.simple_coroot(2))
0
Verify that directionality is correct for roots of different lengths::
sage: lat = RootSystem(['B', 3]).root_lattice()
sage: lat.simple_root(2).scalar(lat.simple_coroot(3))
-2
"""
# Find some better test
if not (lambdacheck in self.parent().coroot_lattice() or lambdacheck in self.parent().coroot_space()):
raise TypeError("%s is not in a coroot lattice/space" % (lambdacheck))
zero = self.parent().base_ring().zero()
cartan_matrix = self.parent().dynkin_diagram()
return sum( (sum( (lambdacheck[i]*s for i,s in cartan_matrix.column(j)), zero) * c for j,c in self), zero)
if lambdacheck in self.parent().coroot_lattice() or lambdacheck in self.parent().coroot_space():
# This is the mathematically canonical case, where we use the Cartan matrix to find the scalar product
zero = self.parent().base_ring().zero()
cartan_matrix = self.parent().dynkin_diagram()
return sum( (sum( (lambdacheck[i]*s for i,s in cartan_matrix.column(j)), zero) * c for j,c in self), zero)

if lambdacheck in self.parent().root_system.ambient_space():
# lambdacheck lives in the ambient space of the root space, so we take the usual dot product in the ambient space
return self.to_ambient().dot_product(lambdacheck)

raise TypeError(f"{lambdacheck} is not in a coroot lattice/space")

def is_positive_root(self):
"""
Expand Down

0 comments on commit 0ac3318

Please sign in to comment.