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

Group cosets: implement custom in, improve iterator #4289

Open
fingolfin opened this issue Nov 7, 2024 · 2 comments
Open

Group cosets: implement custom in, improve iterator #4289

fingolfin opened this issue Nov 7, 2024 · 2 comments
Labels
enhancement New feature or request topic: groups

Comments

@fingolfin
Copy link
Member

Right now in for group cosets resorts to iterating over the coset, which is very inefficient.

We should provide a custom method for this. I would not dispatch to GAP's, but instead implement this "directly": i.e. to test g in Hx, test if g/x in H and similar for g in xH (so that we avoid creating GapObj for those).


Speaking of iterators, the iterate implementation for GroupCoset currently directly wraps a GAP coset iterator. But (a) this requires creating a GapObj (with extra overhead for left cosets). And furthermore on the GAP side, the iterator is a wrapper for an enumerator.

Instead, the Julia iterator for a coset C could be based on the group iterator for C.H.


Lastly, contrast

Base.IteratorSize(::Type{<:GAPGroup}) = Base.SizeUnknown()
Base.IteratorSize(::Type{PermGroup}) = Base.HasLength()

versus

Base.IteratorSize(::Type{<:GroupCoset}) = Base.SizeUnknown()

I suggest we use something like this:

Base.IteratorSize(::Type{<:GroupCoset{T}}) where T = Base.IteratorSize(T)

And then I assume similar for GroupDoubleCoset.

@fingolfin fingolfin added enhancement New feature or request topic: groups labels Nov 7, 2024
@ThomasBreuer
Copy link
Member

For a coset Hg or gH of type GroupCoset{T, S}, T is not necessarily the type of H.
If we want to set Base.IteratorSize for cosets depending on Base.IteratorSize for the type of H then I think we have to change the parameterization to GroupCoset{TG, TH, S} where TG and TH are the types of G and H, respectively.

Concerning the iterator for double cosets, one solution would be to use the functionality of GAP's RepresentativesContainedRightCosets (which is also used by GAP's membership test for double cosets), and to delegate to the iterator for right cosets.

ThomasBreuer added a commit to ThomasBreuer/Oscar.jl that referenced this issue Nov 13, 2024
Add the type of the *subgroup* as a parameter,
then we can prescribe a better `Base.IteratorSize(::Type{<:GroupCoset})`,
as proposed in oscar-system#4289.

Note that in principle, we could omit the type of the *big group* from the
parameters since it is the `parent_type` of the element type parameter.
(The design of the `GroupCoset` type dates back to the times when
we thought that a group has the same type as its subgroups.
At the time when this idea was given up, I should have changed
`GroupCoset` to take the *subgroup* type as a parameter.)
@ThomasBreuer
Copy link
Member

Where are we now:

With #4302, we get

  • in for cosets delegating to in for groups, and in for double cosets delegating to GAP
  • an extended parameterization of the GroupCoset type such that the idea to get a better Base.IteratorSize for cosets works

Concerning double cosets, we would need more GAP functionality on the Oscar side in order to replace the delegations to GAP by something better. For both iteration and membership tests for double cosets, my idea would be to split the double coset into right cosets and to delegate to these object, as is done in GAP. For that, GAP's RepresentativesContainedRightCosets could be exported to Oscar. (I am not sure whether GAP's approach using canonical coset representatives is generically a good idea, it defaults to unpacking right cosets and taking the minimal element.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: groups
Projects
None yet
Development

No branches or pull requests

2 participants