-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
functionality for permuting tensor product order of Qobj #152
Conversation
I believe I addressed all of the comments with this PR, the implementation is totally different. Thanks to @albertomercurio for pointing out a much cleaner way to address this. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #152 +/- ##
==========================================
- Coverage 92.74% 92.68% -0.07%
==========================================
Files 26 26
Lines 1902 1913 +11
==========================================
+ Hits 1764 1773 +9
- Misses 138 140 +2 ☔ View full report in Codecov by Sentry. |
Great! You have to include the missing docstrings in the documentation. Could you also format the documents you changed? |
src/qobj/tensor_functions.jl
Outdated
n_subsystems = length(dims) | ||
|
||
# number of dimensions with size ≠ 1 in original operator (ket, bra, or operator) | ||
# perhaps there's a cleaner way to do this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do multiple dispatch, depending on the type. Such that you can avoid this. You can still follow the ’ptrace` implementation, where it behave differently if it is a Ket or an Operator.
I will clean up this PR today, I also remembered I need to add tests for error handling. |
Great, everything seems fine, except for documentation and format checking. Can you add the function in the documentation, and formatting all the changed files? |
yes! I totally omitted those two issues. |
src/qobj/tensor_functions.jl
Outdated
QO::QuantumObject{<:AbstractArray{T},KetQuantumObject}, | ||
order::AbstractVector{<:Integer} | ||
) where T | ||
if length(order) != length(QO.dims) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before I finish this off, would you like seperate docstrings for each method of permute
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, one general docstring saying that supports both states and operators is fine
BTW, can you remove the JuliaFormatter dependency? We don't need that. You can format the code by just using JuliaFormatter from another environment (or just calling Format Document from vscode) |
src/qobj/tensor_functions.jl
Outdated
@doc raw""" | ||
permute(A::QuantumObject, order::Vector{Int}) | ||
|
||
Permute the subsystems of a [`QuantumObject`](@ref) `A` according to the order specified in `order`. So, if `order = [2, 1, 3]`, the first subsystem will become the second, the second will become the first, and the third will remain the same, i.e. H₁ ⊗ H₂ ⊗ H₃ → H₂ ⊗ H₁ ⊗ H₃. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a more simpler docstring like:
@doc raw"""
permute(A::QuantumObject, order::Vector{Int})
Permute the tensor structure of a [`QuantumObject`](@ref) `A` according to the specified `order` list.
Note that this method currently works for [`Ket`](@ref), [`Bra`](@ref), and [`Operator`](@ref) types of [`QuantumObject`](@ref).
# Examples
If `order = [2, 1, 3]`, the Hilbert space structure will be re-arranged: H₁ ⊗ H₂ ⊗ H₃ → H₂ ⊗ H₁ ⊗ H₃.
\```
julia> ψ1 = fock(2, 0)
julia> ψ2 = fock(3, 1)
julia> ψ3 = fock(4, 2)
julia> ψ_123 = tensor(ψ1, ψ2, ψ3)
julia> permute(ψ_123, [2, 1, 3]) ≈ tensor(ψ2, ψ1, ψ3)
true
\```
"""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(remember to remove the extra \
in the code block under Examples)
src/qobj/tensor_functions.jl
Outdated
``` | ||
|
||
""" | ||
function permute(QO::QuantumObject{<:AbstractArray{T},KetQuantumObject}, order::AbstractVector{<:Integer}) where {T} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest another way to achieve permute
which uses less lines of codes.
Also the check for argument order
can be done by isperm(order)
function permute(
A::QuantumObject{<:AbstractArray{T},ObjType},
order::AbstractVector{Int},
) where {T,ObjType<:Union{KetQuantumObject,BraQuantumObject,OperatorQuantumObject}}
(length(order) != length(A.dims)) &&
throw(ArgumentError("The order list must have the same length as the number of subsystems (A.dims)"))
!isperm(order) && throw(ArgumentError("$(order) is not a valid permutation of the subsystems (A.dims)"))
# obtain the arguments: dims for reshape; perm for PermutedDimsArray
dims, perm = _dims_and_perm(A.type, A.dims, order, length(order))
return QuantumObject(reshape(PermutedDimsArray(reshape(A.data, dims...), perm), size(A)), A.type, A.dims[order])
end
_dims_and_perm(
::ObjType,
dims::AbstractVector{Int},
order::AbstractVector{Int},
L::Int,
) where {ObjType<:Union{KetQuantumObject,BraQuantumObject}} = reverse(dims), reverse!((L + 1) .- order)
_dims_and_perm(::OperatorQuantumObject, dims::AbstractVector{Int}, order::AbstractVector{Int}, L::Int) =
reverse!([dims; dims]), reverse!((2 * L + 1) .- [order; order .+ L])
(the above code has been formatted already)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for putting this together, it's very clean!
test/quantum_objects.jl
Outdated
@test_throws ArgumentError permute(bra_abc, [2, 3, 2]) | ||
|
||
@test_throws ArgumentError permute(op_abc, [2, 3, 1, 4]) | ||
@test_throws ArgumentError permute(op_abc, [2, 3, 2]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to add a more detailed tests, something like:
# permute tests
ket_a = Qobj(rand(ComplexF64, 2))
ket_b = Qobj(rand(ComplexF64, 3))
ket_c = Qobj(rand(ComplexF64, 4))
ket_d = Qobj(rand(ComplexF64, 5))
ket_bdca = permute(tensor(ket_a, ket_b, ket_c, ket_d), [2, 4, 3, 1])
bra_a = ket_a'
bra_b = ket_b'
bra_c = ket_c'
bra_d = ket_d'
bra_bdca = permute(tensor(bra_a, bra_b, bra_c, bra_d), [2, 4, 3, 1])
op_a = Qobj(rand(ComplexF64, 2, 2))
op_b = Qobj(rand(ComplexF64, 3, 3))
op_c = Qobj(rand(ComplexF64, 4, 4))
op_d = Qobj(rand(ComplexF64, 5, 5))
op_bdca = permute(tensor(op_a, op_b, op_c, op_d), [2, 4, 3, 1])
correct_dims = [3, 5, 4, 2]
wrong_order1 = [1]
wrong_order2 = [2, 3, 4, 5]
@test ket_bdca ≈ tensor(ket_b, ket_d, ket_c, ket_a)
@test bra_bdca ≈ tensor(bra_b, bra_d, bra_c, bra_a)
@test op_bdca ≈ tensor(op_b, op_d, op_c, op_a)
@test ket_bdca.dims == correct_dims
@test bra_bdca.dims == correct_dims
@test op_bdca.dims == correct_dims
@test typeof(ket_bdca.type) <: KetQuantumObject
@test typeof(bra_bdca.type) <: BraQuantumObject
@test typeof(op_bdca.type) <: OperatorQuantumObject
@test_throws ArgumentError permute(ket_bdca, wrong_order1)
@test_throws ArgumentError permute(ket_bdca, wrong_order2)
@test_throws ArgumentError permute(bra_bdca, wrong_order1)
@test_throws ArgumentError permute(bra_bdca, wrong_order2)
@test_throws ArgumentError permute(op_bdca, wrong_order1)
@test_throws ArgumentError permute(op_bdca, wrong_order2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it!
@aarontrowbridge |
@ytdHuang thanks! I saw, but I came down with something yesterday and have been rather under the weather. I will try to resolve everything today |
@aarontrowbridge I saw you moved back the |
This pr addresses issue #95. It creates a function
permute
which permutes the subsystem order of compositeKet
,Bra
, andOperator
objects.A new file
src/qobj/tensor_functions.jl
was created to store tensor-related methods.