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

Confusing behaviour in repeated calls of the same code #4656

Closed
ThomasBreuer opened this issue Sep 13, 2021 · 3 comments · Fixed by #4697
Closed

Confusing behaviour in repeated calls of the same code #4656

ThomasBreuer opened this issue Sep 13, 2021 · 3 comments · Fixed by #4697

Comments

@ThomasBreuer
Copy link
Contributor

The following happens in the current master branch as well as in GAP 4.11.1.

I execute some code (essentially from a manual example) in a new GAP session.

gap> G:= Group( (1,2,3,4), (1,2) );;                                           
gap> N:= DerivedSubgroup( G );;
gap> F:= FactorGroupNC( G, DerivedSubgroup( G ) );
Group([ f1 ])
gap> HasParent( F );
true
gap> IsIdenticalObj( F, Parent( F ) );
true

When I enter the same lines again in the same session, the result looks different.

gap> G:= Group( (1,2,3,4), (1,2) );;
gap> N:= DerivedSubgroup( G );;
gap> F:= FactorGroupNC( G, DerivedSubgroup( G ) );
<pc group with 1 generators>
gap> HasParent( F );
false

(I observed this difference because it appears as a difference in manual tests after the changes from pull request #4653.)

Does it make sense at all to set F as its own Parent value in the first case? And if yes then why is the parent not set in the second case (and in all subsequent calls during the session)?

ThomasBreuer added a commit to ThomasBreuer/gap that referenced this issue Sep 14, 2021
(This commit covers just the obvious fixes, not the difference mentioned in issue gap-system#4656.)
@hulpke
Copy link
Contributor

hulpke commented Sep 14, 2021

The issue is basically the function FactorGroup and the GAP3-compatibility that one could call NaturalHomomorphism on a FactorGroup and get the homomorphism.

The factor group itself is the same both times -- GAP caches these small cyclic groups (in CYCLICACHE), since they (and associated families) tend to fill up memory in longer calculations otherwise.

But the second time the group needs to be created anew from its generators (at this point parent is not set, thus the discrepancy), so that different data can be stored for NaturalHomomorphism.

My solution would be to remove this compatibility kludge of first creating the factor group and then asking for its NaturalHomomorphism.

@ThomasBreuer
Copy link
Contributor Author

@hulpke I agree that it is a good idea to get rid of the hack around NaturalHomomorphism and FactorGroup. I hope this functionality is not mentioned in the documentation, and that it is not used in interesting GAP 4 code.

If it is not necessary to create a group anew from the generators of a cached group, this will solve the inconsistency.
However, the question remains why the group stores itself as a parent. What is the benefit of this? My understanding is that one can might get useful information about an object from its stored parent, which is not the case if the object and its parent are identical.
(Note that the actual output that occurs in the manual example in question depends on whether a parent is stored or not.)

@hulpke
Copy link
Contributor

hulpke commented Sep 15, 2021

@ThomasBreuer I suspect it is documented, but to my knowledge it has not been used in code (and all examples I'm aware of have emphasized NaturalHomomorphismByNormalSubgroup.

I think the reason for the stored Parent has been that at some point in the past every PC group was supposed to have a Parent so that HomePcgs was well defined. (This ultimately turned out not to solve the problem, and so is gone now).

hulpke added a commit to hulpke/gap that referenced this issue Nov 12, 2021
hulpke added a commit to hulpke/gap that referenced this issue Nov 12, 2021
hulpke added a commit to hulpke/gap that referenced this issue Nov 12, 2021
Subsequent demotion of `FactorGroup` and related library/manual changes

This resolves gap-system#4656
hulpke added a commit to hulpke/gap that referenced this issue Nov 22, 2021
(Moved the method into obsolete)
Subsequent demotion of `FactorGroup` and related library/manual changes

This resolves gap-system#4656
hulpke added a commit to hulpke/gap that referenced this issue Nov 23, 2021
(Moved the method into obsolete)
Subsequent demotion of `FactorGroup` and related library/manual changes

This resolves gap-system#4656
hulpke added a commit to hulpke/gap that referenced this issue Nov 30, 2021
(Moved the method into obsolete)
Subsequent demotion of `FactorGroup` and related library/manual changes

This resolves gap-system#4656
hulpke added a commit to hulpke/gap that referenced this issue Dec 10, 2021
(Moved the method into obsolete)
Subsequent demotion of `FactorGroup` and related library/manual changes

This resolves gap-system#4656
fingolfin pushed a commit that referenced this issue Dec 10, 2021
(Moved the method into obsolete)
Subsequent demotion of `FactorGroup` and related library/manual changes

This resolves #4656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants