Avoid ambiguities with individual number element (identity)

General Usage
1

I have a question concerning the overloading of operators for an own type

In our package on manifolds, we also cover Lie groups. These have an identity element (see Manifolds.jl/group.jl at 7e90c455804311f1220bcb52878fa5abf86e0abb · JuliaManifolds/Manifolds.jl · GitHub) where we parametrise it by the group under consideration.

Usually we do not type our group elements, since we want to be open for other types. We usually think of matrices representing a point on the manifold, but other types are welcome, too.

Now when overloading operators (e.g. isapprox, but more prominently +,- or *,/ - depending on the group action) we have definitions like (see Manifolds.jl/group.jl at 7e90c455804311f1220bcb52878fa5abf86e0abb · JuliaManifolds/Manifolds.jl · GitHub).

Base.:+(e::Identity{G}) where {G<:AdditionGroup} = e
Base.:+(p::Identity{G}, ::Identity{G}) where {G<:AdditionGroup} = p
Base.:+(::Identity{G}, p) where {G<:AdditionGroup} = p
Base.:+(p, ::Identity{G}) where {G<:AdditionGroup} = p
Base.:+(e::E, ::E) where {G<:AdditionGroup,E<:Identity{G}} = e
  1. How can I avoid ambiguities with other packages that also define a + this way, for example if we would like to use ChainRules (https://github.com/JuliaDiff/ChainRulesCore.jl/blob/22c7bc368354428f0b62c350f1301e24e5644922/src/differential_arithmetic.jl#L137-L143)?
    I am not sure what the implications of this ambiguity (so whether it is important to resolve this) are and how often these appear
  2. One possible ambiguity might appear if one +es two identities from different groups. This is not a valid operation. Would it be reasonable to provide a proper error message instead of the (very vague) Ambiguous error one would get?

Is there some coding style / good practice for these?

2 Likes
2
  1. Just define the methods listed in the ambiguity warning. Metaprogramming is frequently used for code like this (look for for ... @eval evamples).
  2. I would recommend a custom error hint, see Base.Experimental.register_error_hint.
3

The package InitialValues also defines custom identity elements. From what I understand they use Aqua to test that there are no ambiguities. Maybe that could be useful in your case as well.

2 Likes
4

I’m a bit surprised that you decided to introduce an identity type for your groups. Wouldn’t it be easier if you instead relied on the one(x) function to provide the identity element for your groups? If so, you wouldn’t have to define your own addition methods and all the problems in your original post disappear.

6 Likes
5

I thought about that, too; I will check what the effects will be then.

6

This seems to be the correct solution, that will then be our long term goal; thanks for pointing this out. We still have to check where we actually need this type currently (i.e. where we really dispatch on what would be without that the value).

2 Likes