Magma/Group confusion

[hulpke]

This is something that came up in the context of #2387, but really is an earlier problem that has nothing to do with these changes:

Start the master branch (from scratch and with -A to minimize complications) and add the following function (which makes some basic deductions from an objects order):

InstallOtherMethod(SetSize,true,[IsObject and IsAttributeStoringRep,IsObject],
  100, # override system setter
function(obj,sz)
  SetIsEmpty(obj,sz=0);
  SetIsTrivial(obj,sz=1);
  SetIsNonTrivial(obj,sz<>1);
  SetIsFinite(obj,sz<>infinity);
  TryNextMethod(); # to enforce size setting
end);

Now call (this is tested in one of the test files...)

AsGroup([(1,2)]);

and get an error message:

Error, Value property is already set the other way

The reason is that the calculation defines the empty submagma of the magma defined by [(1,2)].
Properties and implication make this submagma a group and thus it contains the trivial permutation, so is not empty (and has Size 1) but GAP has set before that it is empty. The new setter method just makes this visible.

What is at issue here? Code for Magmas? The implication that they could be groups? Or is it the test for AsGroup?

An easy fix would be to take the test out, but there might be some bug being hidden in the current master branch already.

Puzzled...

[hulpke]

The same happens in the tutorial example

gap> mats:= [ [ [ 0*Z(2), Z(2)^0 ],
>               [ Z(2)^0, 0*Z(2) ] ],
>             [ [ Z(2)^0, 0*Z(2) ],
>               [ 0*Z(2), Z(2)^0 ] ] ];; 
gap> Size( AdditiveMagma( mats ) );

[stevelinton]

Got it, I think.

gap> x := MagmaByGenerators([(1,2)]);
<group with 1 generator>
gap> s := Submagma(x,[]);
<group with 0 generators>
gap>    

s here is wrong, it should be the empty set and not a group. I'm not sure where this implication is coming from, but I would guess that there an InstallSubsetMaintenance call somewhere that forgot about this case.

[hulpke]

I think this is more subtle: PermutationsFamily implies IsPerm which in turn implies IsMultiplicativeElementWithInverse. Thus the empty submagma gets an WithInverse from the family and is thus recognized as a group.

I think this fundamentally means that one cannot create an empty submagma consisting of permutations, i.e. the calculations tried actually are not possible.

Maybe @ThomasBreuer has an idea?

[fingolfin]

This sounds related to #2240

[ThomasBreuer]

Technically, the wrong type comes from the implication

IsFiniteOrderElementCollection and IsMagma => IsMagmaWithInverses

in lib/magma.gd, because any collection of elements in PermutationsFamily
(hence also the empty collection)
lies in the filter IsFiniteOrderElementCollection.

The GAP library supports empty domains (which can be regarded as asking for trouble).
If we do not forbid empty domains then the above implication is wrong.

The implication could be replaced by the weaker one

IsFiniteOrderElementCollection and IsMagmaWithOne => IsMagmaWithInverses

If GAP would support antifilters like IsNonempty then one could have

IsFiniteOrderElementCollection and IsMagma and IsNonempty => IsMagmaWithInverses

[fingolfin]

I am actually working on extended implications and "anti implications", see issue #235, and was recently motivated to pick that up again by @hulpke's quest to replace immediate methods; I guess this issue is another motivation.

[stevelinton]

Thanks for digging that out. I wonder if IsNonEmpty is viable? I can't think of a domain which doesn't know from construction if it's non-empty. Anything with a non-zero number of generators, anything in IsMagmaWithOne or IsAdditiveMagma are always non-empty.

[ThomasBreuer]

I agree that almost all domain constructors can easily decide from the given generators and the filters to be set whether the domain in question is empty or not.
(Well, one could imagine a domain representing some set theoretic construction like intersection, union, difference of some collections;
for such a structure, deciding emptyness can be impossible --take the difference of a finitely presented group and the set containing its identity.)
In order to actually store the information about emptyness, all such functions in the GAP library
and in packages have to be extended.

Note:

• IsAdditiveMagma is the additive counterpart to IsMagma, such domains can be empty; IsAdditiveMagmaWithZero implies being nonempty.

• The function SubadditiveMagmaNC erroneously sets the filter IsTrivial (it should set IsEmpty) if the given list of generators is empty, this is just an error to be fixed.

[hulpke]

@ThomasBreuer
Thank you very much I have put a fix for SubadditiveMagmaNC into #2387 as well as a workaround (don't use the empty submagma, but the submagma generated by () ) so that this PR tests w/o errors.
If there is desire, I could put these two changes into a separate PR instead.

[fingolfin]

Yes, please put them into a separate PR. That will make it much easier to review and merge them, and also simplify writing the release notes later (which already now is a major burden, and gets worse when a PR contains two many disparate changes).

[hulpke]

@fingolfin OK, I've put it into #2429
I cannot add any specific test, as the tests are already there but because of the ``quiet ignore'' setting documented for SetSize with a different order are supposed to be quiet and only get triggered with the new setter method from #2387

[fingolfin]

Re tests: perhaps we should add an option which changes our "quiet ignores" rule for attribute setters to "loudly complain with a warning" ? (If that triggers to many warnings, we could refine it to only complain if an equality test fails)

[hulpke]

@fingolfin
Yes, the ``quietly ignore'' policy does not seem to be well-tought out.