Skip to content

Conversation

@wilfwilson
Copy link
Member

@wilfwilson wilfwilson commented Sep 5, 2025

To me it seems like doing something like FreeGroup("x", "x"); can only possibly lead to potential confusion, I can't see a use case for it and I think it should be disallowed. But I'm open to contrary opinions.

To add:

Plus:

  • Add tests for everything
  • Update manual entries for everything
  • Update manual examples

@wilfwilson wilfwilson added do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) topic: library release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Sep 5, 2025
@wilfwilson wilfwilson force-pushed the ww/add-checks-for-duplicates branch from 6d01a95 to 3e0b8e5 Compare September 5, 2025 17:54
@wilfwilson
Copy link
Member Author

It seems there are manual examples that explicitly use duplicate generators, see for example the part of the manual titled "Comparison of Associative Words" (?Comparison of Associative Words), which gives the example:

gap> f:= FreeGroup( "a", "b", "b" );;
gap> gens:= GeneratorsOfGroup(f);
[ a, b, b ]
gap> gens[2] = gens[3];
false
gap> x:= gens[1]*gens[2];
a*b
gap> y:= gens[2]/gens[2]*gens[1]*gens[2];
a*b
gap> x = y;
true
gap> z:= gens[2]/gens[2]*gens[1]*gens[3];
a*b
gap> x = z;
false

Which demonstrates that x <> z, i.e. a * b <> a * b.

We can also see that AssignGeneratorVariables has the following behaviour with duplicate generator names:

gap> G := FreeGroup("a", "b", "b");
<free group on the generators [ a, b, b ]>
gap> AssignGeneratorVariables(G);
#I  Global variable `b' is already defined and will be overwritten
#I  Assigned the global variables [ a, b, b ]

@ChrisJefferson
Copy link
Contributor

I am tempted to forbid it, because I find it hard to imagine someone is using this on purpose, and hasn't just made a typo and is confusing themselves.

@ThomasBreuer
Copy link
Contributor

I agree with @ChrisJefferson.
I think the manual example quoted above should be changed.

My guess is that the idea of this manual example is to demonstrate that words which look equal need not be equal.
However, artificially choosing the same name for different generators is not a good example for that lesson, comparing elements from different free objects would make more sense.

@james-d-mitchell
Copy link
Contributor

I agree with @ThomasBreuer and @ChrisJefferson

@wilfwilson wilfwilson force-pushed the ww/add-checks-for-duplicates branch from 3e0b8e5 to e287507 Compare September 16, 2025 22:11
@wilfwilson wilfwilson marked this pull request as draft September 16, 2025 22:21
@wilfwilson wilfwilson changed the title Disallow duplicate named generators when creating free objects such as FreeMagma, FreeGroup, FreeMonoid, etc Disallow duplicate named generators when constructing magmas, semigroups, groups, etc Sep 17, 2025
@wilfwilson wilfwilson force-pushed the ww/add-checks-for-duplicates branch 2 times, most recently from b113d15 to d35871e Compare September 17, 2025 15:52
@wilfwilson wilfwilson force-pushed the ww/add-checks-for-duplicates branch from d35871e to 04b4fa9 Compare September 17, 2025 16:16
@fingolfin
Copy link
Member

I'd be fine with this as well.

@wilfwilson
Copy link
Member Author

I'll try to get it finished soon.

@limakzi
Copy link
Member

limakzi commented Nov 25, 2025

I agree with @ChrisJefferson, @james-d-mitchell and @fingolfin.
I doubt, I can find a use-case to use same name for two different generators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants