Changes for TensorKitSectors v0.3 #290
Conversation
borisdevos
changed the title
|
I added some changes I made in #263 which are unrelated to multifusion categories or the factorisation tests, the former coming soon in a future PR and the latter because I've been told because of a MatrixAlgebraKit overhaul that the tests may be updated. |
…concerning sectors + some more unitspace
lkdvos
changed the title
| Base.zero(V::ElementarySpace) = zero(typeof(V)) | ||
| zerospace(V::ElementarySpace) = zerospace(typeof(V)) | ||
| Base.zero(V::ElementarySpace) = zerospace(V) | ||
| Base.zero(::Type{V}) where {V <: ElementarySpace} = zerospace(V) |
There was a problem hiding this comment.
I am actually wondering whether this was also considered type piracy. In the same way as overloading Base.show(::Type{MyType}) is not a good idea, this has the same issue, that it is overloading a base method for Type{MyType}, and I don't know if a package is considered to be the owner of Type{MyType} just because it owns MyType. Though I don't see how packages implementing new numerical types could do without something like this.
There was a problem hiding this comment.
Also, this is a non-blocking philosophical question. No action for this PR required.
There was a problem hiding this comment.
Just to share some of my ideas on the topic: I've also been going back and forth on this issue...
I think one way of answering this (definitely not the only way) is to think about who owns something like Vector{MyType} and then claim that actually Type{MyType} might be the same?
For practical reasons, I think it's probably reasonable to "take ownership" of Vector{MyType} for your own number type if you have a specialized implementation for eg. +, but by that logic you should also be allowed to implement show(::Vector{MyType}), and therefore also show(::Type{MyType}).
I think that really, there's two points to type piracy. On the one hand, there is the issue with composing it across packages, you don't want to have multiple people trying to overwrite the same method. On the other hand, there's the issue with composing it in functionality, where you need the behavior of the method to remain the same as what the function intended, as otherwise you might get errors, or even silently wrong results.
The first point is clearly not an issue for things like Vector{MyType} or Type{MyType}, since you own the type so you are "the first" to be able to define a method. The second point is a bit more subtle I think, since I would argue that for +(::Vector{MyType}, ::Vector{MyType}), it is clear what the method should and shouldn't do, while show is just one of those functions where the API is really badly specified, which makes it a lot more dangerous to touch. I think that if the observable behavior of show would be clearly defined, we should be allowed to overload that.
|
This looks good to me. I've slightly reorganized the |
3 participants
This PR introduces the renamings concerning units of sectors, as well as replacing
conjof sectors withdual. Additionally,Base.oneunitnow falls back tounitspace. I also took the liberty of definingzerospace, to whichBase.zerofalls back. This is a breaking change.A follow-up PR will then actually use
UnitStyleetc to generalise a couple of functions to deal with multifusion categories.Something I can still do is update the documentation related to sectors, but I know @Jutho just started on a documentation overhaul, so I haven't done so yet.
This requires QuantumKitHub/TensorKitSectors.jl#25 to be released.