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

Implement Milnor fiber and Coxeter complexes #35038

Merged

Conversation

tscrim
Copy link
Collaborator

@tscrim tscrim commented Feb 9, 2023

Implements Milnor fiber and Coxeter complexes and their associated posets. #34936

Dependencies: #34912

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2023

Codecov Report

Base: 88.59% // Head: 88.59% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (b52defc) compared to base (84eebf0).
Patch coverage: 96.96% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #35038    +/-   ##
=========================================
  Coverage    88.59%   88.59%            
=========================================
  Files         2140     2140            
  Lines       396961   397093   +132     
=========================================
+ Hits        351671   351804   +133     
+ Misses       45290    45289     -1     
Impacted Files Coverage Δ
src/sage/schemes/toric/sheaf/klyachko.py 92.43% <ø> (ø)
...age/categories/finite_complex_reflection_groups.py 83.84% <95.29%> (+4.72%) ⬆️
src/sage/categories/finite_coxeter_groups.py 95.85% <100.00%> (+1.00%) ⬆️
src/sage/groups/additive_abelian/qmodnz.py 91.48% <0.00%> (-2.13%) ⬇️
src/sage/combinat/combination.py 93.50% <0.00%> (-1.50%) ⬇️
src/sage/doctest/forker.py 80.24% <0.00%> (-1.48%) ⬇️
src/sage/groups/generic.py 88.34% <0.00%> (-0.68%) ⬇️
src/sage/graphs/graph_plot.py 84.33% <0.00%> (-0.57%) ⬇️
src/sage/combinat/constellation.py 91.18% <0.00%> (-0.41%) ⬇️
src/sage/groups/cactus_group.py 98.89% <0.00%> (-0.37%) ⬇️
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tscrim
Copy link
Collaborator Author

tscrim commented Sep 12, 2023

@fchapoton Would you be willing to review this?

@fchapoton
Copy link
Contributor

I can try, but I cannot really check the code.

@tscrim
Copy link
Collaborator Author

tscrim commented Sep 13, 2023

Thank you. No problem if you can't. I can ask Vic Reiner to see if he would be willing to review the mathematical portion. I can also split off the Coxeter group portion too.

@tscrim tscrim force-pushed the public/combinat/coxeter_milnor_complex-34936 branch from b52defc to 0b126de Compare September 14, 2023 02:04
@fchapoton
Copy link
Contributor

one linter is not green, about E305

@tscrim
Copy link
Collaborator Author

tscrim commented Sep 14, 2023

one linter is not green, about E305

Done, although the linter spec is not well-defined here as it is about blanklines before a class.

@fchapoton
Copy link
Contributor

Looks good. Is there no way to avoid looping over all elements ?

Too bad that the posets gets huge very fast, so that one cannot even hope to reach H4 (62881 elts) or D5 (12483 elts). They seem to have nice Coxeter polynomials, by the way.

@tscrim
Copy link
Collaborator Author

tscrim commented Sep 14, 2023

The number of maximal elements is equal to the number of elements in the group, so we have to iterate over all elements in some fashion. I might be able to rework things so that we are building the group elements out, but this seems more technical and I am not convinced it will be faster in the end.

@github-actions
Copy link

Documentation preview for this PR (built with commit 6d9ee05; changes) is ready! 🎉

Copy link
Contributor

@fchapoton fchapoton left a comment

Choose a reason for hiding this comment

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

ok, let's get this in.

@tscrim
Copy link
Collaborator Author

tscrim commented Sep 15, 2023

Thank you!

@vbraun vbraun merged commit 8c04027 into sagemath:develop Sep 16, 2023
17 of 18 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Sep 16, 2023
@tscrim tscrim deleted the public/combinat/coxeter_milnor_complex-34936 branch September 18, 2023 07:53
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.

None yet

6 participants