make +One() add the multiplicative identity #60
Conversation
make +One() add the multiplicative identity
| strong_one(primal::AbstractMatrix) = LinearAlgebra.I | ||
| strong_one(primal) = one(primal) | ||
|
|
||
| Base.:+(a::One, b::One) = extern(a) + extern(b) # This is not well-defined. |
There was a problem hiding this comment.
i'd like this comment to be more informative, if possible (i.e "so why do we have this?!")
There was a problem hiding this comment.
Its not changed by this PR.
There was a problem hiding this comment.
It really should be discussed somewhere else,
Maybe #56
Descibing why this is a problem in the code,
is splitting information that belongs in issues.
Maybe i should just revert the change to this line.
| Base.:+(a::One, b::One) = extern(a) + extern(b) # This is not well-defined. | |
| Base.:+(a::One, b::One) = extern(a) + extern(b) |
Co-Authored-By: Nick Robinson <npr251@gmail.com>
Co-Authored-By: Nick Robinson <npr251@gmail.com>
| strong_one(primal::Number) = true | ||
| strong_one(primal::AbstractMatrix) = I | ||
| strong_one(primal) = one(primal) | ||
|
|
|
@willtebbutt is going to just delete |
| @@ -67,11 +67,15 @@ for T in (:One, :AbstractThunk, :Any) | |||
| end | |||
|
|
|||
|
|
|||
| Base.:+(a::One, b::One) = extern(a) + extern(b) | |||
| strong_one(primal::Number) = true | |||
There was a problem hiding this comment.
It has been pointed out by @simeonschaub that if we do
strong_one(primal::Number) = I
this work out well.
Which solves a bunch of problems of not knowing what the right strong(1) is, and kinda removes the need for a strong_one function in the first place?
There was a problem hiding this comment.
No, not really. Strong one probably shouldn't change the number type.
julia> I * 1.5
UniformScaling{Float64}
1.5*I
This is what was suggested in
#56 (comment)
What do we think?