Quantum Equation types. #16

Merged
merged 1 commit into from Jul 11, 2015

Conversation

Projects
None yet
2 participants
@amitjamadagni
Member

amitjamadagni commented Jun 30, 2015

Quantum Equation types added.
WIP : Tests.

src/propexpmsolvers.jl
+
+Step Propagation using the exponential solver Expokit.expmv.
+""" ->
+immutable QuExpm_Expo <: QuExpo

This comment has been minimized.

@acroy

acroy Jul 2, 2015

Contributor

I think underscores in type names isn't a good style. I would say QuExpokit and QuExpmV are better (also QuExponential is better than QuExpo). Remember that we have to make sense out of this in a couple of months.

@acroy

acroy Jul 2, 2015

Contributor

I think underscores in type names isn't a good style. I would say QuExpokit and QuExpmV are better (also QuExponential is better than QuExpo). Remember that we have to make sense out of this in a couple of months.

This comment has been minimized.

@amitjamadagni

amitjamadagni Jul 2, 2015

Member

@acroy I have addressed this in #13

@amitjamadagni

amitjamadagni Jul 2, 2015

Member

@acroy I have addressed this in #13

src/propexpmsolvers.jl
+function propagate(prob::QuExpm_Expo, eq::QuantumEquation, t, current_t, current_qustate)
+ dt = t - current_t
+ next_state = Expokit.expmv(dt, -im*coeffs(operator(eq)), coeffs(vec(current_qustate)), m = get(prob.options, :m, 30), tol = get(prob.options, :tol, 1e-7))
+ return QuArray(next_state)

This comment has been minimized.

@acroy

acroy Jul 2, 2015

Contributor

This is probably not right. Since expmv will return a vector your will construct a QuVector here independent of the type of the initial state. I think you need to store the shape of the state coefficient matrix and do reshape before you can create the QuArray. It would also be better to use QuBase.same_type to preserve the state type instead of always returning a QuArray.

@acroy

acroy Jul 2, 2015

Contributor

This is probably not right. Since expmv will return a vector your will construct a QuVector here independent of the type of the initial state. I think you need to store the shape of the state coefficient matrix and do reshape before you can create the QuArray. It would also be better to use QuBase.same_type to preserve the state type instead of always returning a QuArray.

src/propexpmsolvers.jl
+ dt = t - current_t
+ next_state = ExpmV.expmv(dt, -im*coeffs(operator(eq)), coeffs(vec(current_qustate)), M = get(prob.options, :M, []), prec = get(prob.options, :prec, "double"),
+ shift = get(prob.options, :shift, false), full_term = get(prob.options, :full_term, false))
+ return QuArray(next_state)

This comment has been minimized.

@acroy

acroy Jul 2, 2015

Contributor

see comment above

@acroy

acroy Jul 2, 2015

Contributor

see comment above

src/propmachinery.jl
-immutable QuPropagator{QPM<:QuPropagatorMethod}
- hamiltonian
- init_state::QuVector
+abstract QuantumEquation

This comment has been minimized.

@acroy

acroy Jul 2, 2015

Contributor

I think we should stick to the naming convention we used so far and call this QuEquation. Maybe the other types also should get a Qu-prefix?

@acroy

acroy Jul 2, 2015

Contributor

I think we should stick to the naming convention we used so far and call this QuEquation. Maybe the other types also should get a Qu-prefix?

src/propmachinery.jl
+ nb = size(coeffs(h), 1)
+ SI = Array(Int,0)
+ SJ = Array(Int,0)
+ Lvals = Array(Complex128,0)

This comment has been minimized.

@acroy

acroy Jul 2, 2015

Contributor

Here we will need something like typeof(im*one(eltype(h))) instead of Complex128 to be general.

@acroy

acroy Jul 2, 2015

Contributor

Here we will need something like typeof(im*one(eltype(h))) instead of Complex128 to be general.

src/propmachinery.jl
+ end
+ end
+ end
+ return QuArray(sparse(SI, SJ, Lvals, nb*nb, nb*nb))

This comment has been minimized.

@acroy

acroy Jul 2, 2015

Contributor

Maybe use QuBase.same_type here?

@acroy

acroy Jul 2, 2015

Contributor

Maybe use QuBase.same_type here?

src/propmachinery.jl
- hamiltonian
- init_state::QuVector
+immutable QuPropagator{QPM<:QuPropagatorMethod, QVM<:Union(QuBase.AbstractQuVector,QuBase.AbstractQuMatrix)}
+ eq

This comment has been minimized.

@acroy

acroy Jul 3, 2015

Contributor

Should we make this a subtype of QuEquation?

@acroy

acroy Jul 3, 2015

Contributor

Should we make this a subtype of QuEquation?

This comment has been minimized.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

@acroy I have tried to address this but this would allow cases like
QuSchrodingerEq and AbstractQuMatrix, QuLiouvillevonNeumannEq and AbstractQuVector both of which can be brought to the constructs already present by constructing the superoperator from the hamiltonian of the QuSchrodinderEq in the former and converting the AbstractQuVector to AbstractQuMatrix in the latter. Any thoughts on this would be helpful.

EDIT : Sorry, I did not check the following case

QuPropagator(sigmax, qv*qv', t, QuExpm_ExpmV())

which is the failing case.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

@acroy I have tried to address this but this would allow cases like
QuSchrodingerEq and AbstractQuMatrix, QuLiouvillevonNeumannEq and AbstractQuVector both of which can be brought to the constructs already present by constructing the superoperator from the hamiltonian of the QuSchrodinderEq in the former and converting the AbstractQuVector to AbstractQuMatrix in the latter. Any thoughts on this would be helpful.

EDIT : Sorry, I did not check the following case

QuPropagator(sigmax, qv*qv', t, QuExpm_ExpmV())

which is the failing case.

This comment has been minimized.

@acroy

acroy Jul 3, 2015

Contributor

I think I don't understand your doubts? We only store subtypes of QuEquation, which might be created in the constructor.

@acroy

acroy Jul 3, 2015

Contributor

I think I don't understand your doubts? We only store subtypes of QuEquation, which might be created in the constructor.

This comment has been minimized.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

@acroy I have passed QuEquation type onto eq, which is in the latest commit. Please do let me know if I am missing something here.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

@acroy I have passed QuEquation type onto eq, which is in the latest commit. Please do let me know if I am missing something here.

src/propstepsolvers.jl
- basis_size = get(prob.options,:NC, length(current_qustate))
- N = min(basis_size, length(current_qustate))
- v = Array(typeof(current_qustate),0)
+ basis_size = get(prob.options,:NC, length(vec(current_qustate)))

This comment has been minimized.

@acroy

acroy Jul 3, 2015

Contributor

I think it might be cleaner to define psi=vec(current_state) at the beginning of the function and then only use that.

@acroy

acroy Jul 3, 2015

Contributor

I think it might be cleaner to define psi=vec(current_state) at the beginning of the function and then only use that.

src/propstepsolvers.jl
alpha = Array(Complex{Float64},0)
@compat sizehint!(alpha, N)
beta = Array(Complex{Float64},0)
@compat sizehint!(beta, N+1)
push!(beta,0.)
for i=2:N
- w = op*v[i]
+ w = operator(eq)*v[i]

This comment has been minimized.

@acroy

acroy Jul 3, 2015

Contributor

Similarly, it would be good to have op=operator(eq) before the loop (since we can't assume that operator won't be expensive).

@acroy

acroy Jul 3, 2015

Contributor

Similarly, it would be good to have op=operator(eq) before the loop (since we can't assume that operator won't be expensive).

src/quequations.jl
+
+export SchrodingerEquation,
+ LiouvillevonNeumannEquation,
+ liouvillian_op,

This comment has been minimized.

@acroy

acroy Jul 3, 2015

Contributor

Not sure if we should export liouvillian_op and liouvillian_tensor.

@acroy

acroy Jul 3, 2015

Contributor

Not sure if we should export liouvillian_op and liouvillian_tensor.

This comment has been minimized.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

@acroy I guess these have been exported for use in the constructs from propmachinery.jl, may be placing them in propmachinery.jl may avoid this.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

@acroy I guess these have been exported for use in the constructs from propmachinery.jl, may be placing them in propmachinery.jl may avoid this.

This comment has been minimized.

@acroy

acroy Jul 3, 2015

Contributor

Where are they used in propmachinery.jl? But even if they are, there is no need to export them since this file and propmachinery.jl are in the same module.

@acroy

acroy Jul 3, 2015

Contributor

Where are they used in propmachinery.jl? But even if they are, there is no need to export them since this file and propmachinery.jl are in the same module.

This comment has been minimized.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

We have used them in the following construct :

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix}(param::QuBase.AbstractQuMatrix, init_state::QM,  tlist, method::QPM) = QuPropagator{QPM,QM}(QuLiouvillevonNeumannEq(liouvillian_op(param)),init_state, tlist, method)

in propmachinery.jl.

But even if they are, there is no need to export them since this file and propmachinery.jl are in the same module.

Ah! okay.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

We have used them in the following construct :

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix}(param::QuBase.AbstractQuMatrix, init_state::QM,  tlist, method::QPM) = QuPropagator{QPM,QM}(QuLiouvillevonNeumannEq(liouvillian_op(param)),init_state, tlist, method)

in propmachinery.jl.

But even if they are, there is no need to export them since this file and propmachinery.jl are in the same module.

Ah! okay.

src/quequations.jl
@@ -0,0 +1,54 @@
+abstract QuEquation
+
+immutable SchrodingerEquation{H<:QuBase.AbstractQuMatrix} <: QuEquation

This comment has been minimized.

@acroy

acroy Jul 3, 2015

Contributor

Can I convince you to rename those types again using Qu as prefix, i.e. Qu SchrodingerEquation (or Qu SchrodingerEq) and QuLiouvillevonNeumannEquation (or QuLiouvillevonNeumannEq)?

@acroy

acroy Jul 3, 2015

Contributor

Can I convince you to rename those types again using Qu as prefix, i.e. Qu SchrodingerEquation (or Qu SchrodingerEq) and QuLiouvillevonNeumannEquation (or QuLiouvillevonNeumannEq)?

This comment has been minimized.

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

I forgot this :( .. thanks for reminding

@amitjamadagni

amitjamadagni Jul 3, 2015

Member

I forgot this :( .. thanks for reminding

src/propmachinery.jl
- hamiltonian
- init_state::QuVector
+immutable QuPropagator{QPM<:QuPropagatorMethod, QVM<:Union(QuBase.AbstractQuVector,QuBase.AbstractQuMatrix)}
+ eq::QuEquation

This comment has been minimized.

@acroy

acroy Jul 4, 2015

Contributor

It is actually better to use a type parameter (as you did before). Using abstract types for fields is discouraged.

@acroy

acroy Jul 4, 2015

Contributor

It is actually better to use a type parameter (as you did before). Using abstract types for fields is discouraged.

This comment has been minimized.

@amitjamadagni

amitjamadagni Jul 4, 2015

Member

@acroy I tried something like this

immutable QuPropagator{QPM<:QuPropagatorMethod, QVM<:Union(QuBase.AbstractQuVector,QuBase.AbstractQuMatrix), QE<:Union(QuEquation,QuBase.AbstractQuMatrix)}
    eq::QE
    init_state::QVM
    tlist
    method::QPM
    QuPropagator(eq, init_state, tlist, method) = new(eq, init_state, tlist, method)
end

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector, QE<:QuSchrodingerEq}(eq::QE, init_state::QV, tlist, method::QPM) = QuPropagator{QPM,QV,QE}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector, QE<:QuBase.AbstractQuMatrix}(param::QE, init_state::QV,  tlist, method::QPM) = QuPropagator{QPM,QV,QE}(QuSchrodingerEq(param),init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix, QE<:QuLiouvillevonNeumannEq}(eq::QE, init_state::QM, tlist, method::QPM) = QuPropagator{QPM,QM,QE}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix, QE<:QuBase.AbstractQuMatrix}(param::QE, init_state::QM,  tlist, method::QPM) = QuPropagator{QPM,QM,QE}(QuLiouvillevonNeumannEq(liouvillian_op(param)),init_state, tlist, method)

I end up with a convert error

ERROR: `convert` has no method matching convert(::Type{QuArray{FiniteBasis{Orthonormal},Float64,2,Array{Float64,2}}}, ::QuLiouvillevonNeumannEq{QuArray{FiniteBasis{Orthonormal},Complex{Float64},2,SparseMatrixCSC{Complex{Float64},Int64}}})

QE also needs to support two types QuEquation and AbstractQuMatrix as we need for the constructs 2 and 4 in the above. Any thoughts on this would be helpful.

@amitjamadagni

amitjamadagni Jul 4, 2015

Member

@acroy I tried something like this

immutable QuPropagator{QPM<:QuPropagatorMethod, QVM<:Union(QuBase.AbstractQuVector,QuBase.AbstractQuMatrix), QE<:Union(QuEquation,QuBase.AbstractQuMatrix)}
    eq::QE
    init_state::QVM
    tlist
    method::QPM
    QuPropagator(eq, init_state, tlist, method) = new(eq, init_state, tlist, method)
end

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector, QE<:QuSchrodingerEq}(eq::QE, init_state::QV, tlist, method::QPM) = QuPropagator{QPM,QV,QE}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector, QE<:QuBase.AbstractQuMatrix}(param::QE, init_state::QV,  tlist, method::QPM) = QuPropagator{QPM,QV,QE}(QuSchrodingerEq(param),init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix, QE<:QuLiouvillevonNeumannEq}(eq::QE, init_state::QM, tlist, method::QPM) = QuPropagator{QPM,QM,QE}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix, QE<:QuBase.AbstractQuMatrix}(param::QE, init_state::QM,  tlist, method::QPM) = QuPropagator{QPM,QM,QE}(QuLiouvillevonNeumannEq(liouvillian_op(param)),init_state, tlist, method)

I end up with a convert error

ERROR: `convert` has no method matching convert(::Type{QuArray{FiniteBasis{Orthonormal},Float64,2,Array{Float64,2}}}, ::QuLiouvillevonNeumannEq{QuArray{FiniteBasis{Orthonormal},Complex{Float64},2,SparseMatrixCSC{Complex{Float64},Int64}}})

QE also needs to support two types QuEquation and AbstractQuMatrix as we need for the constructs 2 and 4 in the above. Any thoughts on this would be helpful.

This comment has been minimized.

@acroy

acroy Jul 5, 2015

Contributor

The <:Union(QuEquation,QuBase.AbstractQuMatrix) is not necessary, we always convert to QuSchrodingerEq etc and store that in QuPropagator. So QuPropagator{QPM<:QuPropagatorMethod, QVM<:Union(QuBase.AbstractQuVector,QuBase.AbstractQuMatrix), QE<:QuEquation} will do.

Regarding the constructors you can try

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector}(eq::QuSchrodingerEq, init_state::QV, tlist, method::QPM) = QuPropagator{QPM,QV,QuSchrodingerEq}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector}(param::QuBase.AbstractQuMatrix, init_state::QV,  tlist, method::QPM) = QuPropagator{QPM,QV,QuSchrodingerEq}(QuSchrodingerEq(param),init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix}(eq::QuLiouvillevonNeumannEq, init_state::QM, tlist, method::QPM) = QuPropagator{QPM,QM,QuLiouvillevonNeumannEq}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix}(param::QuBase.AbstractQuMatrix, init_state::QM,  tlist, method::QPM) = QuPropagator{QPM,QM,QuLiouvillevonNeumannEq}(QuLiouvillevonNeumannEq(liouvillian_op(param)),init_state, tlist, method)

And it might make sense to specify the types of the parameters in the inner constructor:
QuPropagator(eq::QE, init_state::QVM, tlist, method::QPM).

@acroy

acroy Jul 5, 2015

Contributor

The <:Union(QuEquation,QuBase.AbstractQuMatrix) is not necessary, we always convert to QuSchrodingerEq etc and store that in QuPropagator. So QuPropagator{QPM<:QuPropagatorMethod, QVM<:Union(QuBase.AbstractQuVector,QuBase.AbstractQuMatrix), QE<:QuEquation} will do.

Regarding the constructors you can try

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector}(eq::QuSchrodingerEq, init_state::QV, tlist, method::QPM) = QuPropagator{QPM,QV,QuSchrodingerEq}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QV<:QuBase.AbstractQuVector}(param::QuBase.AbstractQuMatrix, init_state::QV,  tlist, method::QPM) = QuPropagator{QPM,QV,QuSchrodingerEq}(QuSchrodingerEq(param),init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix}(eq::QuLiouvillevonNeumannEq, init_state::QM, tlist, method::QPM) = QuPropagator{QPM,QM,QuLiouvillevonNeumannEq}(eq, init_state, tlist, method)

QuPropagator{QPM<:QuPropagatorMethod, QM<:QuBase.AbstractQuMatrix}(param::QuBase.AbstractQuMatrix, init_state::QM,  tlist, method::QPM) = QuPropagator{QPM,QM,QuLiouvillevonNeumannEq}(QuLiouvillevonNeumannEq(liouvillian_op(param)),init_state, tlist, method)

And it might make sense to specify the types of the parameters in the inner constructor:
QuPropagator(eq::QE, init_state::QVM, tlist, method::QPM).

src/propexpmsolvers.jl
+function propagate(prob::QuExpokit, eq::QuEquation, t, current_t, current_qustate)
+ dt = t - current_t
+ dims = size(current_qustate)
+ next_state = Expokit.expmv(dt, -im*coeffs(operator(eq)), coeffs(current_qustate), m = get(prob.options, :m, 30), tol = get(prob.options, :tol, 1e-7))

This comment has been minimized.

@acroy

acroy Jul 4, 2015

Contributor

Don't you need vec here?

@acroy

acroy Jul 4, 2015

Contributor

Don't you need vec here?

@acroy

This comment has been minimized.

Show comment
Hide comment
@acroy

acroy Jul 4, 2015

Contributor

Are there any tests for the Liouville-Von Neumann equation?

Contributor

acroy commented Jul 4, 2015

Are there any tests for the Liouville-Von Neumann equation?

@amitjamadagni

This comment has been minimized.

Show comment
Hide comment
@amitjamadagni

amitjamadagni Jul 4, 2015

Member

@acroy vec edits done, tests added for Liouville-Von Neumann equation (they seem to fail with an error liouvillian_op is not defined). The subtyping of eq is still to be resolved but I have included the ways in the line note. Any thoughts on this would be helpful.

Member

amitjamadagni commented Jul 4, 2015

@acroy vec edits done, tests added for Liouville-Von Neumann equation (they seem to fail with an error liouvillian_op is not defined). The subtyping of eq is still to be resolved but I have included the ways in the line note. Any thoughts on this would be helpful.

test/propagatortest.jl
+@test_approx_eq coeffs(next_state_quexpm_expo[1][2]) coeffs(next_state_actual)
+@test_approx_eq coeffs(next_state_quexpm_expmv[1][2]) coeffs(next_state_actual)
+
+lvn = QuLiouvillevonNeumannEq(liouvillian_op(sigmax))

This comment has been minimized.

@acroy

acroy Jul 5, 2015

Contributor

With my constructors you won't need liouvillian_op here.

EDIT: I take this back. You will need it, but must call it like QuDynamics.liouvillian_op.

@acroy

acroy Jul 5, 2015

Contributor

With my constructors you won't need liouvillian_op here.

EDIT: I take this back. You will need it, but must call it like QuDynamics.liouvillian_op.

@amitjamadagni

This comment has been minimized.

Show comment
Hide comment
@amitjamadagni

amitjamadagni Jul 5, 2015

Member

@acroy the changes are done. Also I have worked on the Lindblad master equation types, can I commit it here or can we have a separate PR for it ?

Member

amitjamadagni commented Jul 5, 2015

@acroy the changes are done. Also I have worked on the Lindblad master equation types, can I commit it here or can we have a separate PR for it ?

@acroy

This comment has been minimized.

Show comment
Hide comment
@acroy

acroy Jul 5, 2015

Contributor

Looks better. Lindblad equation should get its own PR (generally, it is a good idea to have many small PRs instead of few big ones. This makes reviewing easier and problems can be identified more easily).

Contributor

acroy commented Jul 5, 2015

Looks better. Lindblad equation should get its own PR (generally, it is a good idea to have many small PRs instead of few big ones. This makes reviewing easier and problems can be identified more easily).

@amitjamadagni

This comment has been minimized.

Show comment
Hide comment
@amitjamadagni

amitjamadagni Jul 5, 2015

Member

@acroy is this ready to go, we still need to add the additional constructors for QuLiouvillevonNeumannEq, but could we first go for the implementation of lindblad master equation and back port as we discussed ?

Member

amitjamadagni commented Jul 5, 2015

@acroy is this ready to go, we still need to add the additional constructors for QuLiouvillevonNeumannEq, but could we first go for the implementation of lindblad master equation and back port as we discussed ?

src/propmachinery.jl
t
t_state
end
+function operator(qu_eq::QuLiouvillevonNeumannEq)

This comment has been minimized.

@acroy

acroy Jul 6, 2015

Contributor

I think operator belongs into quequations.jl.

@acroy

acroy Jul 6, 2015

Contributor

I think operator belongs into quequations.jl.

src/propodesolvers.jl
-for op in keys(type_to_method)
- text = type_to_method[op]
+for op in keys(type_to_method_ode)
+ text = type_to_method_ode[op]

This comment has been minimized.

@acroy

acroy Jul 6, 2015

Contributor

Maybe we should call this method_name instead of text?

@acroy

acroy Jul 6, 2015

Contributor

Maybe we should call this method_name instead of text?

+
+QuLiouvillevonNeumannEq{H<:QuBase.AbstractQuMatrix}(liouvillian::H) = QuLiouvillevonNeumannEq{H}(liouvillian)
+
+function liouvillian_op(h::QuBase.AbstractQuMatrix)

This comment has been minimized.

@acroy

acroy Jul 6, 2015

Contributor

This function will need some description (and comments), but we can do this later in connection with the Lindblad type.

@acroy

acroy Jul 6, 2015

Contributor

This function will need some description (and comments), but we can do this later in connection with the Lindblad type.

src/propstepsolvers.jl
dt = t - current_t
- return (eye(op)-im*op*dt)*current_qustate
+ return (eye(operator(eq))-im*operator(eq)*dt)*vec(current_qustate)

This comment has been minimized.

@acroy

acroy Jul 6, 2015

Contributor

It is better to do op = operator(eq) before this statement and then only use op. This way you only call operator once.

@acroy

acroy Jul 6, 2015

Contributor

It is better to do op = operator(eq) before this statement and then only use op. This way you only call operator once.

src/propstepsolvers.jl
dt = t - current_t
- uni = eye(op)-im*op*dt/2
- return \(uni', uni*current_qustate)
+ uni = eye(operator(eq))-im*operator(eq)*dt/2

This comment has been minimized.

@acroy

acroy Jul 6, 2015

Contributor

Same as above regarding operator.

@acroy

acroy Jul 6, 2015

Contributor

Same as above regarding operator.

@amitjamadagni

This comment has been minimized.

Show comment
Hide comment
@amitjamadagni

amitjamadagni Jul 10, 2015

Member

@acroy a review would be helpful. I have added the density matrix support for step solvers and ode solvers. I will open the PR on QuLindbladMasterEq types and constructs soon. I am currently working on writing more tests.

Member

amitjamadagni commented Jul 10, 2015

@acroy a review would be helpful. I have added the density matrix support for step solvers and ode solvers. I will open the PR on QuLindbladMasterEq types and constructs soon. I am currently working on writing more tests.

@acroy

This comment has been minimized.

Show comment
Hide comment
@acroy

acroy Jul 10, 2015

Contributor

Looks good. You should at least add one test for the Liouville problem with an ODE solver, so we can see that this works as well.

Contributor

acroy commented Jul 10, 2015

Looks good. You should at least add one test for the Liouville problem with an ODE solver, so we can see that this works as well.

@amitjamadagni

This comment has been minimized.

Show comment
Hide comment
@amitjamadagni

amitjamadagni Jul 10, 2015

Member

@acroy done! I guess we could merge this ?

Member

amitjamadagni commented Jul 10, 2015

@acroy done! I guess we could merge this ?

@acroy

This comment has been minimized.

Show comment
Hide comment
@acroy

acroy Jul 10, 2015

Contributor

Yes (after squashing). I think the tests need some more work (for instance now there is no check if the calculations for the density matrices are actually correct), but this can be done later.

Contributor

acroy commented Jul 10, 2015

Yes (after squashing). I think the tests need some more work (for instance now there is no check if the calculations for the density matrices are actually correct), but this can be done later.

Quantum Equation types and constructs.
a. QuSchrodingerEq
b. QuLiouvillevonNeumannEq

amitjamadagni added a commit that referenced this pull request Jul 11, 2015

Merge pull request #16 from amitjamadagni/eq_types
Quantum Equation types and constructs
a. QuSchrodingerEq
b. QuLiouvillevonNeumannEq

@amitjamadagni amitjamadagni merged commit 3dfc746 into JuliaQuantum:master Jul 11, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@amitjamadagni

This comment has been minimized.

Show comment
Hide comment
@amitjamadagni

amitjamadagni Jul 11, 2015

Member

#13, #14 have been included in this pull. #13 has an updated README that will go in a new PR. #13, #14 have been closed as they have been included.

Member

amitjamadagni commented Jul 11, 2015

#13, #14 have been included in this pull. #13 has an updated README that will go in a new PR. #13, #14 have been closed as they have been included.

@amitjamadagni amitjamadagni deleted the amitjamadagni:eq_types branch Jul 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment