Skip to content

Hyperbolic Coxeter type classification added #40343

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

SamyMekkati
Copy link

This pull request introduces a classification of Coxeter matrices based on the hyperbolic examples provided in Humphreys' book Reflection Groups and Coxeter Groups (Chapter 6, Appendix, e.g., page 142). The aim is to integrate these specific examples into SageMath as a new Coxeter type by adding the class "CoxeterType_Hyperbolic" and some functions defined in type_hyperbolic.py

📝 Checklist

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

⌛ Dependencies

@SamyMekkati
Copy link
Author

@jplab

@@ -51,6 +52,9 @@ def __classcall_private__(cls, *x):

if isinstance(x, CoxeterType):
return x

if isinstance(x,list) and x[0] == "Hyp" or x[0] == "Hyperbolic":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add one space after comma and before "list"

@@ -185,8 +193,13 @@ def _samples(self):
['C', 5, 1], ['D', 5, 1], ['E', 6, 1],
['E', 7, 1], ['E', 8, 1], ['F', 4, 1],
['G', 2, 1], ['A', 1, 1]]]

hyperbolic = [CoxeterType(t) for t in [['Hyp', (141, 1, 3)], ['Hyp', (141, 1, 4)],
['Hyp', (141, 2, 5)], ['Hyp', (142, 1, 6)],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

align the lines

sage: CoxeterType(['Hyp', (141, 1, 3)]).is_hyperbolic()
True
"""

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line here

from sage.combinat.root_system.coxeter_type import CoxeterType

"""
Hyperbolic Coxeter matrices for hyperbolic Coxeter types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no indentation ; one empty line after first line

"""
hyperbolic_coxeter_matrices = {
(141, 1, 1): CoxeterMatrix([
[1,4,2,2],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spaces after commas, everywhere in this file


return self._position


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one empty line between methods inside a class

def is_hyperbolic(self):
"""
Return ``True`` since ``self`` is a hyperbolic Coxeter type.
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a doctest

sage: C = CoxeterType(["Hyp",(143, 1, 6)])
sage: C.index_set()
(1, 2, 3, 4, 5, 6)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no empty line at the end of docs


def is_affine(self):
"""
Return ``False`` since all hyperbolic coxeter graph is not affine.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong grammar, mix of singular and plural; same in the next method

def is_crystallographic(self):
"""
Return whether ``self`` is crystallographic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just on empty line here, please

Copy link

github-actions bot commented Jul 1, 2025

Documentation preview for this PR (built with commit 9d73a4d; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@fchapoton
Copy link
Contributor

fchapoton commented Jul 1, 2025

you also need to add the newly added file to the list in the file meson.build in the same folder

@jplab
Copy link
Contributor

jplab commented Jul 1, 2025

Hi Frédéric,

I asked Samy to add this classification as a starter ticket. I wanted to tag you, but you were faster than me!

Then, the plan would be to add a recognition function for these. It might be quite involved, but I think it would be great...

@jplab jplab requested review from jplab and tscrim July 1, 2025 14:29
@fchapoton
Copy link
Contributor

Salut Jean-Philippe. Fait trop chaud, alors j'essaye de faire des choses pas trop compliquées..

Si tu as une seconde, peux-tu regarder jplab/brocoli#6

@fchapoton
Copy link
Contributor

another question : how does this relate to the tables in McMullen's article : https://www.numdam.org/item/PMIHES_2002__95__151_0.pdf ?

@fchapoton fchapoton changed the title Hyperbolic coxeter type classification added Hyperbolic Coxeter type classification added Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants