Use @varnames_interface free_group

[fingolfin]

• enables many variants of the free_group argument syntax
• adds @free_group macro

Implements part of #3125

The main issue is the changed return value. I did not bother to adjust all tests or documentation in this PR to that, the changes I made are enough to illustrate the pros and cons I think: returning the group and generators is nice and natural in some cases, and ugly and unnatural in others (just like with polynomial_ring I guess ;-) ).

But the major concern I have about it is that it now differs from all other (GAP based?) group constructors. Of course all of these could also be made to return generators, but this change is more and more breaking.

So right now my thought would be to drop this part and let free_group only return the group. OK? Thoughts?

To make this possible, we'll have to either tweak @varnames_interface a bit, or else use a little trickery (like: rename free_group to _free_group, then let free_group just pass all its arguments to _free_group.).

This looses any ability to retain the "grouping" of variables when using free_group, i.e., there would be no direct replacement for F, x, y = free_group(:x => 1:3, :y => 1:3).

However, it would be retained for the macro version:

julia> F = @free_group(:x => 1:3, :y => 1:3)
Free group of rank 6

julia> x
3-element Vector{FPGroupElem}:
 x[1]
 x[2]
 x[3]

julia> y
3-element Vector{FPGroupElem}:
 y[1]
 y[2]
 y[3]

[thofma]

I find my self always typing F = free_group(["x", "y"]); x, y = gens(F), so I would welcome the change of the return type. But I have no strong opinion and I am also probably not one of the main customer for this functionality.

[fingolfin]

I guess free_group is some sense much closer to polynomial_ring or free_associative_algebra: for all of these one often wants to work with combinations of "words" expressed in terms of their generators, so it can be useful to ask for those. Which after all is the very reason why I am considering using @varnames_interface on free_groups !

The difference with symmetric_group and matrix groups is that there I do this much less frequently.

So I could see some justification to handle free_group and symmetric_group differently in this regard. But it's still not great. Hrm.

But I'd like to decouple that problem from this PR, so I'll look into the _free_group approach for now.

[fingolfin]

This is now ready for review (regardless of whether it passes the test suite or not, I'll fix that up later if necessary).

In particular I've written a comprehensive docstring for free_group and a smaller one for @free_group. Feedback highly welcome -- perhaps the text I wrote there could be reused to improve the docstring for polynomial_ring which doesn't really explain the available options.

[lgoettgens]

I think this is going in a great direction. I added some comments to things I find weird. And CI is unhappy that you broke free_group() without any arguments

[lgoettgens]

Is it intentional that you didn't change it here?

[fingolfin]

yes

[lgoettgens]

Why is this different? Is this something you really want to have or is it just very hard to work around?

[fingolfin]

This was back then agreed upon for the behavior of @varnames_interface as users from algebraic geometry wanted e.g. @polynomial_ring(QQ, :x => (1:2,1:2)) to produce variables x12 etc. not a matrix x.

Honestly I am not that happy with it anymore. I would prefer if we had

• @polynomial_ring(QQ, "x" => (1:2,1:2)) defines in the current scope a 2x2 matrix x
• @polynomial_ring(QQ, "x#" => (1:2,1:2)) defines the current scope four variables x11,x12,x21,x22

Then hopefully usage would still be convenient enough for users from algebraic geometry? I'll ask @wdecker when I am back from vacation.

[fingolfin]

(In particular this is not something I can reasonably change in this PR)

[ThomasBreuer]

Nice.

There is a cross-reference from free_group to @free_group, shouldn't @free_group be in the manual?

[codecov]

Codecov Report

Attention: Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.58%. Comparing base (6caa552) to head (fb42471).
Report is 8 commits behind head on master.

Files	Patch %	Lines
src/Groups/group_constructors.jl	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3317      +/-   ##
==========================================
+ Coverage   84.56%   84.58%   +0.02%     
==========================================
  Files         597      597              
  Lines       82197    82246      +49     
==========================================
+ Hits        69508    69566      +58     
+ Misses      12689    12680       -9     

Files	Coverage Δ
experimental/GModule/src/Brueckner.jl	85.71% <ø> (ø)
src/Groups/GAPGroups.jl	94.13% <ø> (ø)
src/Groups/group_constructors.jl	92.08% <92.30%> (-1.00%)	⬇️

... and 11 files with indirect coverage changes