canonical_unit implementations differ from docs

[lgoettgens]

I thought about adding some conformance checks for canonical_unit via adding

@test is_unit(canonical_unit(a))
@test equality(canonical_unit(a * b), canonical_unit(a) * canonical_unit(b))

after this line.
The reasoning comes from the documentation.

However, this fails for basically all rings, and is even worse for some others (cf. #1166). The main reason being canonical_unit(0 * something) = canonical_unit(0) = 1 != 1 * canonical_unit(something) = canonical_unit(0) * canonical_unit(something).

To circumvent this problem, one could change the conformance test to

@test is_unit(canonical_unit(a))
@test iszero(a * b) || equality(canonical_unit(a * b), canonical_unit(a) * canonical_unit(b))

This still fails for some rings, e.g. Matrix rings, due to the implementation there (link to implementation).

I would suggest either making the implementations compliant with the documentation or modifying the documentation.
Any thoughts on this?

[thofma]

Thanks for summing up the mismatch between the documentation and the reality. There are some other aspects of canonical_unit that require changes that would including fixing this. So I am not sure it is a good idea to spend time on fixing this.

(For example, the function should return the inverse of what it returns now and/or the caonicalized value. It should also be optional, probably using a trait like CanCanonicalize (like it is done in the iterator interface https://docs.julialang.org/en/v1/manual/interfaces/). But this now turns into a bigger overhaul that no one has time right now.)

[fieker]

Where is it stated that canonical_unit is a morphism? I don't think it is..

…

On Sun, 29 Jan 2023, 22:25 Tommy Hofmann, ***@***.***> wrote: Thanks for summing up the mismatch between the documentation and the reality. There are some other aspects of canonical_unit that require changes that would including fixing this. So I am not sure it is a good idea to spend time on fixing this. (For example, the function should return the inverse of what it returns now and/or the caonicalized value. It should also be optional, probably using a trait like CanCanonicalize (like it is done in the iterator interface https://docs.julialang.org/en/v1/manual/interfaces/). But this now turns into a bigger overhaul that no one has time right now.) — Reply to this email directly, view it on GitHub <#1254 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA36CV4BPVKRHD34V3MSILDWU3N45ANCNFSM6AAAAAAUKNHBV4> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[fieker]

Found the counter example:z/8[X]: 6x+1 gets normalised by 3 to 2x+3 Multiply by 4, normalised by 1... you get 4 normalised by 1 again

…

On Mon, 30 Jan 2023, 06:54 Claus Fieker, ***@***.***> wrote: Where is it stated that canonical_unit is a morphism? I don't think it is.. On Sun, 29 Jan 2023, 22:25 Tommy Hofmann, ***@***.***> wrote: > Thanks for summing up the mismatch between the documentation and the > reality. There are some other aspects of canonical_unit that require > changes that would including fixing this. So I am not sure it is a good > idea to spend time on fixing this. > > (For example, the function should return the inverse of what it returns > now and/or the caonicalized value. It should also be optional, probably > using a trait like CanCanonicalize (like it is done in the iterator > interface https://docs.julialang.org/en/v1/manual/interfaces/). But this > now turns into a bigger overhaul that no one has time right now.) > > — > Reply to this email directly, view it on GitHub > <#1254 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AA36CV4BPVKRHD34V3MSILDWU3N45ANCNFSM6AAAAAAUKNHBV4> > . > You are receiving this because you are subscribed to this thread.Message > ID: ***@***.***> >

[fieker]

OK, reading the doc, I think what is required/ suggested is that for a unit u, we have that
canonical_unit(a)u == canonical_unit(au) whic his trivially canonical_unit(a) * canonical_unit(u) since for units canonical_unit(u) == u always.

In general this is tricky: for a Ring R we consider a system of representatives of R modulo units and canonical_unit more of less picks the rep. So canonical_unit(u) == u just means that one rep is 1.
In geenral, I don't think one can impose any homomorphic properties on the representatives chosen....

[fingolfin]

@lgoettgens where in the docs does it claim that canonical_unit is a homomorphism? I just couldn't find a place, but if there is one, it should indeed be fixed.

In the meantime, docs/src/ring_interface.md describes the identity @fieker mentions, where one factor is a unit.

[fieker]

On Fri, Mar 10, 2023 at 12:05:49AM -0800, Max Horn wrote: @lgoettgens where in the docs does it claim that `canonical_unit` is a homomorphism? I just couldn't find a place, but if there is one, it should indeed be fixed.

I don't think it can be a homomorphism at all: R = Z/12Z then canonical_unit(R(2)) = 1 since gcd(12, 2) = 2 = 1*2 then canonical_unit(R(4)) = 1 since gcd(12, 4) = 4 = 1*4 then canonical_unit(R(8)) = 5 since gcd(12, 8) = 4 = 5*8

…

In the meantime, `docs/src/ring_interface.md` describes the identity @fieker mentions, where one factor is a unit. -- Reply to this email directly or view it on GitHub: #1254 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[lgoettgens]

@lgoettgens where in the docs does it claim that canonical_unit is a homomorphism? I just couldn't find a place, but if there is one, it should indeed be fixed.

In the meantime, docs/src/ring_interface.md describes the identity @fieker mentions, where one factor is a unit.

It comes from the same place in stable/ring_interface/#Canonicalisation @fieker mentioned. I think I read it that way since it is not stated that u is supposed to be a unit.