lack of API for checking left and right bases

[Krastanov]

Most of the user-facing basis-checking API (like using basis to get the basis in which an operator works) is convenient only for square matrices. If we have different left and right bases, one needs to use the .basis_r and .basis_l fields, which does not compose well with anything outside of the QuantumOptics package (e.g. QuantumSymbolics and QuantumClifford do not have such fields in their types).

We need something like basis_r and basis_l functions.

And we need to be careful what such functions do to superoperators.

[akirakyle]

As part of qojulia/QuantumOpticsBase.jl#115 and qojulia/QuantumOpticsBase.jl#177 I've been dealing a lot with the choices made around bases. Here's some thoughts based on issues I've run into with the current interface.

First the current abstract type interface for the quantum primitives looks like:

abstract type Basis end

function basis end

abstract type StateVector{B,T} end
abstract type AbstractKet{B,T} <: StateVector{B,T} end
abstract type AbstractBra{B,T} <: StateVector{B,T} end
abstract type AbstractOperator{BL,BR} end
abstract type AbstractSuperOperator{B1,B2} end

It's a little strange to me to have only the abstract type StateVector and its abstract subtypes have a T type that is supposed to represent the type of the underlying data while AbstractOperator and AbstractSuperOperator do not. The T type of the underlying data is only introduced in QuantumOpticsBase via the typing of their structs.

So I think a potentially better abstract interface would probably look like this:

abstract type Basis end
abstract type OperatorBasis{BL<:Basis,BR<:Basis} end
abstract type SuperOperatorBasis{BL<:OperatorBasis,BR<:OperatorBasis} end

function basis end
function basis_l end
function basis_r end

abstract type StateVector{B<:Basis,T} end
abstract type AbstractKet{B,T} <: StateVector{B,T} end
abstract type AbstractBra{B,T} <: StateVector{B,T} end
abstract type AbstractOperator{B<:OperatorBasis,T} end
abstract type AbstractSuperOperator{B<:SuperOperatorBasis, T} end

Thus every quantum object must explicitly carry the appropriate number of bases. This fixes the current situation with AbstractSuperOperator where in QuantumOpticsBase B1 and B2 can end up being a tuple or vector of an arbitrary number of bases (although only the first two are ever used, and qojulia/QuantumOpticsBase.jl#115 enforces that there's only ever two elements in B1 and B2). Furthermore, basis_l and basis_r can then make sense for both AbstractOperator and AbstractSuperOperator where the former will return a subtype of Basis while the latter will return a subtype of OperatorBasis. While somewhat unsatisfactory from a principled point of view, basis could continue it's overloaded meaning of returning the underlying Basis for both AbstractOperator and AbstractSuperOperator only when they are all the same basis and throw IncompatibleBases otherwise since that is usually what one wants.

I realize this would be a breaking change, but I'm willing to go through the work to implement it or anything else which improves the current situation, especially since it is very relevant for my ongoing work on the Choi and and Kraus superoperator representations.
P.S. @Krastanov I think it's fine to hold off on review of qojulia/QuantumOpticsBase.jl#115 if it's easier for you to review all the superoperator stuff together, and especially if changes to the abstract interface are made as a result of this issue.

[akirakyle]

ping @Krastanov! I'm done with the code and tests in qojulia/QuantumOpticsBase.jl#177 and just have all the documentation tasks remaining. I would prefer to address this issue though before doing all the documentation as I think it would affect the documentation for those PRs.

[Krastanov]

Apologies for the slow response. Your reminder in the other issue is what put this back on my radar.

I think I feel favorably towards this. I am wondering whether we can make it less breaking.

• Currently basis errors if the operators (or superoperators) are not square. What if we keep that behavior and introduce fullbasis (or some similarly named) that has the behavior you are suggesting? -- thus nonbreaking and we can release it immediately

• I agree about getting the bases a bit more organized in the type parameters. But why is Tuple{Basis, Basis} worse than OperatorBasis{Basis,Basis}? What are the tradeoffs here?

• I do not like that the Abstract* operators specify a numerical data type T. I like how currently we have AbstractOperator and DataOperator and only the latter specifies the numerical data type. This becomes important when we are dealing with symbolic objects. This is something we probably need to discuss more -- currently QuantumSymbolics works fine, but would it be easier to maintain it if we have AbstractDataKet and AbstractKet be separate entities?

[akirakyle]

Responding to each point you raise:

• I agree that at this point we should probably keep the current behavior of basis to be non-breaking as changing all the downstream uses would be a huge pain with little clear benifit. I like the idea of introducing a fullbasis operator to simultaneously get basis_l and basis_r. If we have a fullbasis function, separate basis_l and basis_r functions would then not be necessary.
• I think having, say, AbstractSuperOperator{B<:Tuple{Tuple{Basis, Basis}, Tuple{Basis, Basis}}, T} is functionally equivalent and fixes my main issue with the current superoperator basis type being underspecified. The main reason I can think to prefer the named abstract types would be clarity when instances of these are printed. There may also be some benefit to preventing confusion with the .bases field of a CompositeBasis, however I think such confusion wouldn't really happen in practice. Meanwhile using tuples has the advantage of reducing the number of abstract types we introduce. I don't think I have a preference one way or the other.
• I made this issue before taking a deeper looking into QuantumSymbolics.jl and I agree it requires more consideration. I think that given that the T is in practice used in QuantumOpticsBase.jl, but not in QuantumSymbolics.jl, it makes more sense to not specify such a free parametric type at the interface level, but let the downstream packages define it if they need. I don't think it really makes sense to have an AbstractDataKet at the interface level, given quantum optics base already introduces this abstract type for it's own interal use as abstract type DataOperator{BL,BR} <: AbstractOperator{BL,BR} end which also doesn't include the underlying data type T parameter.

To summarize, here's a possible abstract interface with these suggestions:

abstract type Basis end

abstract type StateVector{B<:Basis} end
abstract type AbstractKet{B} <: StateVector{B} end
abstract type AbstractBra{B} <: StateVector{B} end
abstract type AbstractOperator{Tuple{B<:Basis, B<:Basis}} end
abstract type AbstractSuperOperator{Tuple{Tuple{B<:Basis, B<:Basis}, Tuple{B<:Basis, B<:Basis}}} end

# non-breaking keeping same semantics of throwing error if basis_l != basis_r
function basis end 

# returns B where B<:Basis for StateVector, AbstractKet, AbstractBra,
# returns Tuple{B,B} where B<:Basis for AbstractOperator
# returns Tuple{Tuple{B,B}, Tuple{B,B}} where B<:Basis for AbstractSuperOperator
function fullbasis end

[akirakyle]

On the second point, it seems I don't know how to correctly define abstract subtypes of Tuple. The example in my previous comment errors with syntax: invalid variable expression in "where". Any other way I try to write the definition gives similar errors and I think this may have something with the special covariant nature of tuples? (see https://docs.julialang.org/en/v1/devdocs/types/#Tuple-types) So perhaps it's best not to rely on Tuple given it's special typing rules which essentially drive the whole ruleset for multiple dispatch in julia?

Then without tuples, the proposed abstract types look like

abstract type Basis end
abstract type OperatorBasis{BL<:Basis,BR<:Basis} end
abstract type SuperOperatorBasis{BL<:OperatorBasis,BR<:OperatorBasis} end

abstract type StateVector{B<:Basis} end
abstract type AbstractKet{B} <: StateVector{B} end
abstract type AbstractBra{B} <: StateVector{B} end
abstract type AbstractOperator{B<:OperatorBasis} end
abstract type AbstractSuperOperator{B<:SuperOperatorBasis} end

# non-breaking keeping same semantics of throwing error if basis_l != basis_r
function basis end 

# returns B where B<:Basis for StateVector, AbstractKet, AbstractBra,
# returns B where B<:OperatorBasis for AbstractOperator
# returns B where B<:SuperOperatorBasis for AbstractSuperOperator
function fullbasis end

[Krastanov]

Just one more concern here (I think I agree with everything else).

Concerning something like AbstractKet vs Ket and AbstractOperator vs DataOperator vs Operator.

Currently we have Operator{basis,datatype} <: DataOperator{basis} <: AbstractOperator{basis}

julia> a = Operator(GenericBasis(2), rand(2,2))
Operator(dim=2x2)
  basis: Basis(dim=2)
 0.890606  0.2058
 0.809609  0.836239

julia> typeof(a)
Operator{GenericBasis{Vector{Int64}}, GenericBasis{Vector{Int64}}, Matrix{Float64}}

julia> supertype(Operator)
DataOperator

julia> supertype(supertype(Operator))
AbstractOperator

and currently we have Ket{basis, datatype} <: AbstractKet{basis, datatype}

julia> a = Ket(GenericBasis(2), rand(2))
Ket(dim=2)
  basis: Basis(dim=2)
 0.1295987205459196
 0.5131532669813034

julia> typeof(a)
Ket{GenericBasis{Vector{Int64}}, Vector{Float64}}

julia> supertype(Ket)

So if I am getting your suggestion right, you want to skip the equivalent of DataOperator but still remove the dataype from the AbstractKet type parameters (and keep it only in Ket)?

I am fine with that. I guess my first sentence was wrong, this is not a concern, just wanted to make sure we are on the same page.

I am strongly in favor of this. It will also be a good excuse to release QuantumOpticsBase v1 (and to start keeping a CHANGELOG.md like in QuantumSymbolics). If you make these changes I will be championing them in front of the other maintainers (pinging @amilsted , @ChristophHotter , @david-pl )

@amilsted , @ChristophHotter , @david-pl -- the summary is in this current comment and in the previous comments -- all earlier comments are a bit outdated.

This is also technically non-breaking -- changes to type parameters of unexported abstract types is not public API.

[akirakyle]

So if I am getting your suggestion right, you want to skip the equivalent of DataOperator but still remove the dataype from the AbstractKet type parameters (and keep it only in Ket)?

Yes exactly that, I was mainly motivated by making the abstract type signatures for AbstractKet, AbstractOperator, and AbstractOperator all consistent.

I'll definitely have issues #27 and #25 in mind as I work on this!