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

Incorrect implications from IsMapping to IsNearRingElement, resp. IsGeneralMapping to IsNearAdditiveElementWithInverse #12

Open
fingolfin opened this issue Nov 8, 2018 · 12 comments

Comments

@fingolfin
Copy link
Member

The following lines of code in Sonata are incorrect and break other things in GAP (see gap-system/gap#2818)

InstallTrueMethod( IsNearRingElement, IsMapping );

InstallTrueMethod( IsNearAdditiveElementWithInverse, IsGeneralMapping );

They are incorrect because they essentially claim that all (general) mappings are near ring elements resp. near additive elements with inverse, which simply is not true. In particular, in general such mappings cannot be added.

I do not know enough about Sonata to suggest an appropriate fix, other than to remove the offending lines, but clearly that breaks other things in Sonata.

@fingolfin
Copy link
Member Author

@PMayr any thoughts on this?

@PMayr
Copy link
Collaborator

PMayr commented Nov 18, 2018

The intention for these definitions was clearly to capture only mappings whose domain and codomain are the same group, e.g., endomorphisms on a group, in order to be able to define a sum of these mappings.
I don't know how to realize this at this level. The methods installed for +, Zero, AdditiveInverse in grptfms.gi in the Sonata library online work for endo mappings on groups. Can we replace the implication IsMapping -> IsNearRingElement by something that captures that?

@fingolfin
Copy link
Member Author

If you want the same domain and codomain, then, as you probably know IsEndoMapping resp. IsEndoGeneralMapping reflect that. To get mappings between groups, use IsGroupGeneralMapping

So I guess one could use IsGroupGeneralMapping and IsEndoMapping resp. IsGroupGeneralMapping and IsEndoGeneralMapping in the above.

But even there I am a bit sceptical that this is the way to go. For starters, what is the sum of two mappings between multiplicative groups supposed to be?

@PMayr
Copy link
Collaborator

PMayr commented Nov 22, 2018

Since groups in GAP are multiplicative in general, we were sort of forced to define the addition of functions on groups as pointwise multiplication in Sonata. This is in line with the notation of nearrings of functions on groups as structures with addition (interpreted as pointwise multiplication) and multiplication (interpreted as composition of functions).

I followed your suggestion to define

InstallTrueMethod( IsNearAdditiveElementWithInverse, IsGroupGeneralMapping and IsEndoGeneralMapping);

in grptfms.gd and adapted some other filters in grptfms.gi (I've not submitted them to github yet but I can if you want to see them). Anyways, something seems to go wrong with this implication in the example below. Although b is first not recognized as IsNearAdditiveElementWithInverse, it is
in IsGroupGeneralMapping and IsEndoGeneralMapping. Any ideas? Is this worthwhile to pursue?

gap> C := CyclicGroup(2);
<pc group of size 2 with 1 generators>
gap> M := MapNearRing( C );
TransformationNearRing(<pc group of size 2 with 1 generators>)
gap> b := AsSortedList(M)[1];
<mapping: Group( [ f1 ] ) -> Group( [ f1 ] ) >
gap> IsNearAdditiveElementWithInverse(b);
false
gap> IsGroupGeneralMapping(b);
true
gap> IsEndoGeneralMapping(b);
true
gap> IsNearAdditiveElementWithInverse(b);
true

Although b is first not recognized as IsNearAdditive

@fingolfin
Copy link
Member Author

The above output means that b did not "know" it was in both filters IsGroupGeneralMapping and IsEndoGeneralMapping until you asked GAP to check whether that's the case. So to make this work, you'll probably have to modify your MapNearRing function to ensure that all objects in it have these properties set.

@PMayr
Copy link
Collaborator

PMayr commented Nov 28, 2018

Right, thanks. I can make the changes if you think this would rectify the existing problems outside of Sonata. Or is something else needed?

@fingolfin
Copy link
Member Author

That would be great, thank you. But I'll have to see and test the changes to be able to tell if they suffice (but I think this is very likely)

@fingolfin
Copy link
Member Author

@PMayr please let me know when you make the change, then I can test it from this side, and then perhaps a new release with the fix in it can be made.

@fingolfin
Copy link
Member Author

@PMayr any news on this?

@PMayr
Copy link
Collaborator

PMayr commented Jan 17, 2019 via email

@PMayr
Copy link
Collaborator

PMayr commented Feb 12, 2019

Getting back to the problem of the following declarations in sonata:

InstallTrueMethod( IsNearRingElement, IsMapping );
InstallTrueMethod( IsNearAdditiveElementWithInverse, IsGeneralMapping );

As mentioned before these should capture mappings (not necessarily homomorphisms) whose domain and codomain are the same group. So I think IsMapping/IsGeneralMapping could safely be replaced by IsEndoMapping/IsEndoGeneralMapping.
@fingolfin suggested to add IsGroupGeneralMapping as well. I now realize that this filter is for mappings between groups that respect multiplication and inverses, which I certainly don't want. Any unary function on a group should be able to be in IsNearRingElement since we can add them pointwise (using the group multiplication) and compose them (as usual).
Any ideas how to realize this and fix the conflicts of the definitions above with gap?

@fingolfin
Copy link
Member Author

So I thought a bit about this... I think InstallTrueMethod( IsNearRingElement, IsEndoMapping ); is fine. I have some uneasy feeling about InstallTrueMethod( IsNearAdditiveElementWithInverse, IsEndoGeneralMapping ); -- as that implies that every IsEndoGeneralMapping is near additive, which I find rather confusing. But then, I guess this is OK as (a) with your package loaded, well, one can add them; (b) it certainly is not worse than (in fact, an improvement over) what sonata does right now.

So, if that change results in working code for you, that'd be OK, I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants