Adv/algebraic shifting #4260
Adv/algebraic shifting #4260
Conversation
This reverts commit 398d1f9.
…/algebraic-shifting
| """ | ||
| function rothe_matrix(F::Field, w::WeylGroupElem) | ||
| W = parent(w) | ||
| phi = isomorphism(PermGroup, W) |
There was a problem hiding this comment.
Do we guarantee somewhere that this is a specific isomorphism? (@lgoettgens will know)? Otherwise this code here strikes me as potentially problematic.
There was a problem hiding this comment.
No, at the moment we don't. This particular function doesn't even have a docstring yet. When in the future I add a docstring, I either add this guarantee there, or I'll move the current implementation to a different function that will then have this guarantee. Also, as soon as this PR is merged, we will notice things like this in CI
There was a problem hiding this comment.
Did we add the specific isomorphism only for the fp group version?
There was a problem hiding this comment.
Yes, exactly. The iso to FPGroup has some guarantees (number of gens with coxeter relations). The iso to PermGroup is atm only implemented for type A_n with standard ordering and has no guarantees
There was a problem hiding this comment.
OK we can sort it out later, this is code in experimental after all -- I'll look into providing a better / more general isomorphism(PermGroup, ::WeylGroup) method, but perhaps also another function which allows choosing specific permutation representations.
There was a problem hiding this comment.
Thanks for looking into this @fingolfin!
Co-authored-by: Max Horn <[email protected]>
| if show_progress | ||
| edge_labels = reduce((d1, d2) -> mergewith!(vcat, d1, d2), | ||
| @showprogress map_function( | ||
| Ks -> multi_edges(F, phi.(W), Ks, complex_labels), | ||
| Iterators.partition(enumerate(complexes), task_size))) | ||
| else | ||
| edge_labels = reduce((d1, d2) -> mergewith!(vcat, d1, d2), | ||
| map_function( | ||
| Ks -> multi_edges(F, phi.(W), Ks, complex_labels), | ||
| Iterators.partition(enumerate(complexes), task_size))) | ||
| end |
There was a problem hiding this comment.
You could merge these two cases by instead using
edge_labels = reduce((d1, d2) -> mergewith!(vcat, d1, d2),
@showprogress enabled=show_progress map_function(
Ks -> multi_edges(F, phi.(W), Ks, complex_labels),
Iterators.partition(enumerate(complexes), task_size)))since the progress meter has a kwarg to dis/enable it
There was a problem hiding this comment.
thanks, added to #4381
Algebraic shifting first merge into Oscar.
@flenzen @micjoswig