introduce `on_points`, `on_tuples`, ... #127

ThomasBreuer · 2020-07-10T12:34:57Z

This is a first attempt to introduce action functions.
Once it is clear that this idea is o.k.,
more such functions have to follow, more methods have to be provided
(for example on_right for right cosets and group elements),
and then functions for orbits, stabilizers, induced permutations, etc.
can be added.

thofma · 2020-07-10T12:41:07Z

Regarding the syntax, we could additionally overload directly x^g or g(x) respectively x * g and g * x. But this would just be syntactic sugar.

fingolfin · 2020-07-10T12:52:52Z

Thank you! (if you haven't already, please also see PR #119, which I just merged (and which is super boring and nothing much happens, I thought it'd be good for you to be aware), and issue #108, which still lacks details ).

I envision that we will do two things in parallel:

provide on_FOOBAR functions like this PR does
provide a more Julia-like API which exploits the richer types for datastructures: when considering [[1,2], [3,4]]^g in GAP, this is ambiguous, hence we have OnSets, OnTuples, OnSetsSets, OnSetsTuples etc.. In Julia, though, we can use the type: Tuple vs. Set vs. BitSet etc. (and we probably will want some extra types, too: Set is great but for small sets with just a single digits of entries, it might be more efficient to use a "sorted vector" that works more like what GAP does; see e.g. https://github.com/tpapp/SortedVectors.jl or https://github.com/JuliaIntervals/IntervalOptimisation.jl/blob/master/src/SortedVectors.jl ; but actually we could just have our own super simple one which basically just adds a "tag" to the vector that it is meant to be sorted and should be transformed as such by permutations).

In the second approach, what Tommy wrote absolutely makes sense, as we can "automatically" decide for the "correct" action. In fact, I'd just implement the actions as such.

(However, I am not quite sure how to implement g(x) in terms of x^g, other than perhaps by inserting an inversion somewhere; that's about the only generic way to turn a right action into a left action, no?

Anyway, I'll look at this PR after our status meeting :-)

codecov · 2020-07-10T12:53:48Z

Codecov Report

Merging #127 into master will decrease coverage by 0.30%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #127      +/-   ##
==========================================
- Coverage   33.12%   32.81%   -0.31%     
==========================================
  Files          20       21       +1     
  Lines        2992     3023      +31     
==========================================
+ Hits          991      992       +1     
- Misses       2001     2031      +30

Impacted Files	Coverage Δ
src/Groups/GAPGroups.jl	`83.87% <ø> (-0.37%)`	⬇️
src/Groups/action.jl	`0.00% <0.00%> (ø)`
src/Groups/types.jl	`90.32% <ø> (ø)`
src/Oscar.jl	`6.81% <ø> (ø)`

fingolfin · 2020-07-12T20:56:41Z

src/Groups/action.jl

+"""
+ on_tuples(tuple::GAP.GapObj, x::GAPGroupElem)
+ on_tuples(tuple::Vector{T}, x::GAPGroupElem) where T
+ on_tuples(tuple::T, x::GAPGroupElem) where T <: Tuple


How about this instead:

Suggested change

on_tuples(tuple::T, x::GAPGroupElem) where T <: Tuple

on_tuples(tuple::Tuple{Vararg{T}}, x::GAPGroupElem) where T

Going beyond this (and perhaps beyond this PR): as mentioned already, for tuples, it'd also be natural to just define a ^ method directly, as it is "clear" how to act on that. Just as I'd argue that the default action for a vector ought to be OnRight, for a Set or BitSet OnSets etc. -- and then nested, i.e. a Set{Set{Int}} suggest OnSetsSets, while Set{Tuple{Vararg{Int8}}} suggests OnSetsTuples etc.

In my variant, the tuple need not be homogeneous, and it is simpler to create the result object of the right type.

fingolfin · 2020-07-12T21:00:41Z

src/Groups/action.jl

+"""
+on_points(pnt::GAP.GapObj, x::GAPGroupElem) = GAP.Globals.OnPoints(pnt, x.X)
+
+on_points(pnt::GAPGroupElem, x::GAPGroupElem) = inv(x) * pnt * x


That's inefficient, e.g. for permutations, conjugation can be computed much more efficiently than that. So I think it really should be this:

Suggested change

on_points(pnt::GAPGroupElem, x::GAPGroupElem) = inv(x) * pnt * x

on_points(pnt::GAPGroupElem, x::GAPGroupElem) = pnt ^ x

fingolfin · 2020-07-12T21:01:48Z

src/Groups/action.jl

+"""
+ on_points(pnt::GAP.GapObj, x::GAPGroupElem)
+ on_points(pnt::GAPGroupElem, x::GAPGroupElem)
+ on_points(pnt::Int, x::PermGroupElem)


Honest question: What is the purpose in having on_points over just using :^? I.e. where one passes on_points as argument, couldn't one just pass ^ instead? It's quite likely I am missing something, but what? Note that I occasionally wonder the same about GAP.

fingolfin · 2020-07-12T21:02:53Z

src/Groups/action.jl

+"""
+ on_right(pnt::GAP.GapObj, x::GAPGroupElem)
+ on_right(pnt::GAPGroupElem, x::GAPGroupElem)
+ on_right(v::Vector{T}, x::MatrixGroupElem) where T


As for OnPoints / on_points, what is the advantage of having on_right, as compared to just passing :* as "the action" whenever needed?

fingolfin · 2020-07-22T15:13:29Z

@ThomasBreuer I just realized that I actually already left some feedback here, so waiting for your reply here now :-)

fingolfin · 2020-08-04T09:38:04Z

@ThomasBreuer do you plan to update this PR?

ThomasBreuer · 2020-08-04T10:44:53Z

@fingolfin Yes.

This is a first attempt to introduce action functions. Once it is clear that this idea is o.k., more such functions have to follow, more methods have to be provided (for example `on_right` for right cosets and group elements), and then functions for orbits, stabilizers, induced permutations, etc. can be added. For example, `on_right`

- added a description what this is about - define `^` as default action of permutations on (pos.) integers - added natural action of permutations on multivariate polynomials by permuting indeterminates - do not introduce `on_points`, `on_right`

rfourquet · 2020-09-02T12:37:29Z

src/Groups/action.jl

+ on_indeterminates(f::GAP.GapObj, p::PermGroupElem)
+ on_indeterminates(f::Nemo.MPolyElem{T}, p::PermGroupElem) where T
+
+Returns the image of `f` under `p`, w.r.t. permuting the indeterminates


Suggested change

Returns the image of `f` under `p`, w.r.t. permuting the indeterminates

Return the image of `f` under `p`, w.r.t. permuting the indeterminates

Yes, thanks for the hint.

fingolfin

Looks OK to me. I still would like us to also install default methods for ^ for acing on Tuple, Set, etc. "in the usual way", but that doesn't have to hold up this PR. Also, we'll still need dedicated actions anyway. E.g. when acting on a tuple or vector, while I'd consider on_tuples the "default", of course one could also act by permuting the tuple entries (on_positions? on_indices?)

Also, this should be consolidated with the to-be-fleshed-out concept of G-sets, but again, no need to hold this up for now.

Merge this at will?

fieker · 2020-09-04T08:43:04Z

On Fri, Sep 04, 2020 at 01:41:43AM -0700, Max Horn wrote: @fingolfin approved this pull request. Looks OK to me. I still would like us to also install default methods for ^ for acing on `Tuple`, `Set`, etc. "in the usual way", but that doesn't have to hold up this PR. Also, we'll still need dedicated actions anyway. E.g. when acting on a tuple or vector, while I'd consider `on_tuples` the "default", of course one could also act by permuting the tuple entries (`on_positions`? `on_indices`?) Also, this should be consolidated with the to-be-fleshed-out concept of G-sets, but again, no need to hold this up for now. Merge this at will?

Yep. at will....

…

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #127 (review)

ThomasBreuer · 2020-09-04T11:13:30Z

@fingolfin I do not understand your latest comment. The default methods for ^ had been added in the updated version of the pull request, and I had added also an explanation at the beginning of the file why one cannot use ^ in all situations --eventually this has to become part of the documentation, but currently all this is work in progress.

fingolfin · 2020-09-04T11:46:18Z

@ThomasBreuer Sorry for the confusion. Apparently I was looking at an outdated version of the PR. I've force-reloaded and now see those ^ methods. I'll go over this PR once again.

fingolfin · 2020-10-02T13:30:59Z

Closing and re-opening to re-run the tests

fingolfin reviewed Jul 12, 2020

View reviewed changes

ThomasBreuer added 2 commits August 21, 2020 12:11

rfourquet reviewed Sep 2, 2020

View reviewed changes

fingolfin approved these changes Sep 4, 2020

View reviewed changes

fingolfin closed this Oct 2, 2020

fingolfin reopened this Oct 2, 2020

fingolfin merged commit 476dd82 into oscar-system:master Oct 5, 2020

ThomasBreuer deleted the TB_group_action_functions branch June 2, 2022 09:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce `on_points`, `on_tuples`, ... #127

introduce `on_points`, `on_tuples`, ... #127

ThomasBreuer commented Jul 10, 2020

thofma commented Jul 10, 2020

fingolfin commented Jul 10, 2020

codecov bot commented Jul 10, 2020 •

edited

Loading

fingolfin Jul 12, 2020

ThomasBreuer Aug 26, 2020

fingolfin Jul 12, 2020

fingolfin Jul 12, 2020

fingolfin Jul 12, 2020

fingolfin commented Jul 22, 2020

fingolfin commented Aug 4, 2020

ThomasBreuer commented Aug 4, 2020

rfourquet Sep 2, 2020

ThomasBreuer Sep 4, 2020

fingolfin left a comment

fieker commented Sep 4, 2020 via email

ThomasBreuer commented Sep 4, 2020

fingolfin commented Sep 4, 2020

fingolfin commented Oct 2, 2020

	on_tuples(tuple::T, x::GAPGroupElem) where T <: Tuple
	on_tuples(tuple::Tuple{Vararg{T}}, x::GAPGroupElem) where T

	on_points(pnt::GAPGroupElem, x::GAPGroupElem) = inv(x) * pnt * x
	on_points(pnt::GAPGroupElem, x::GAPGroupElem) = pnt ^ x

	Returns the image of `f` under `p`, w.r.t. permuting the indeterminates
	Return the image of `f` under `p`, w.r.t. permuting the indeterminates

introduce on_points, on_tuples, ... #127

introduce on_points, on_tuples, ... #127

Conversation

ThomasBreuer commented Jul 10, 2020

thofma commented Jul 10, 2020

fingolfin commented Jul 10, 2020

codecov bot commented Jul 10, 2020 • edited Loading

Codecov Report

fingolfin Jul 12, 2020

Choose a reason for hiding this comment

ThomasBreuer Aug 26, 2020

Choose a reason for hiding this comment

fingolfin Jul 12, 2020

Choose a reason for hiding this comment

fingolfin Jul 12, 2020

Choose a reason for hiding this comment

fingolfin Jul 12, 2020

Choose a reason for hiding this comment

fingolfin commented Jul 22, 2020

fingolfin commented Aug 4, 2020

ThomasBreuer commented Aug 4, 2020

rfourquet Sep 2, 2020

Choose a reason for hiding this comment

ThomasBreuer Sep 4, 2020

Choose a reason for hiding this comment

fingolfin left a comment

Choose a reason for hiding this comment

fieker commented Sep 4, 2020 via email

ThomasBreuer commented Sep 4, 2020

fingolfin commented Sep 4, 2020

fingolfin commented Oct 2, 2020

introduce `on_points`, `on_tuples`, ... #127

introduce `on_points`, `on_tuples`, ... #127

codecov bot commented Jul 10, 2020 •

edited

Loading