qojulia / QuantumOptics.jl

Library for the numerical simulation of closed as well as open quantum systems.
http://qojulia.org
Other
518 stars 101 forks source link

Adding hint for master solver function #388

Closed a-eghrari closed 1 month ago

a-eghrari commented 2 months ago

Adding hint for master equation solver when there is a MethodError. I also referenced master_dynamic in the see also section of master. #386

Krastanov commented 1 month ago

something weird is happening with the tests on x86 architectures, but it does not seem it is due to this pull request. The issue is during the precompilation of SparseArrays. I do not believe it should block this PR as it is unrelated, but we should at least file an issue and see whether there was potentially a accidental break in a non-breaking release of the SparseArrays library

Krastanov commented 1 month ago

@a-eghrari , is it possible to add some checks against false positives here.

For instance, if someone has completely nonsensical arguments for master the hint will be a bit misleading and should be not shown. Could you make the hint show only if there is an appropriate number of arguments (3 or 4) and the second argument is already correct (of type Operator)?

a-eghrari commented 1 month ago

@Krastanov, Thanks for your explanation.

Sure, that is feasible. In addition, I can also type-check the third argument against something like Union{Function, AbstractTimeDependentOperator} so we can then be sure that the user is using the wrong function to solve time evolution. What do you think of that?

Upon going through examples and source code, I have noticed that both master and Shrodinger solvers are equipped with a check method (_check_const) which throws an ArgumentError If the user uses subtypes of AbstractTimeDependentOperator for Hamiltonian. Even though I see the method is_const is dispatched for the Function type as well, it seems it does not work since master and Shrodinger solvers have not been dispatched on Functions. For example (This is example is from QuantumOptics.jl'd documentation):

using QuantumOptics

basis = SpinBasis(1//2)
ψ₀ = spindown(basis)
const sx = sigmax(basis)
function H_pump(t, psi)
  return sin(t)*sx
end

H_pump_tdop = TimeDependentSum(sin=>sx)

const J = [sigmam(basis)]
const Jdagger = [sigmap(basis)]
function f(t,rho)
    H = sin(t)*sx
    return H, J, Jdagger
end

tspan = [0:0.1:10;]

tout, ψₜ = timeevolution.schroedinger_dynamic(tspan, ψ₀, H_pump) 
#Works fine
tout, ψₜ = timeevolution.schroedinger(tspan, ψ₀, H_pump) 
# Throws a MethodError 

tout, ψₜ = timeevolution.schroedinger_dynamic(tspan, ψ₀, H_pump_tdop) 
#Works fine
tout, ψₜ = timeevolution.schroedinger(tspan, ψ₀, H_pump_tdop)
# Throws an ArgumentError with the message: "You are attempting to use a time-dependent dynamics generator (a Hamiltonian or Lindbladian) with a solver that assumes constant dynamics. To avoid errors, please use the _dynamic solvers instead, e.g. schroedinger_dynamic instead of schroedinger"

tout, ρₜ = timeevolution.master_dynamic(tspan, ψ₀, f)
#Works fine
tout, ρₜ = timeevolution.master(tspan, ψ₀, f)
# Throws a MethodError 
tout, ρₜ = timeevolution.master(tspan, ψ₀, H_pump_tdop, J)
# Throws an ArgumentError with the message: "You are attempting to use a time-dependent dynamics generator (a Hamiltonian or Lindbladian) with a solver that assumes constant dynamics. To avoid errors, please use the _dynamic solvers instead, e.g. schroedinger_dynamic instead of schroedinger"
tout, ρₜ = timeevolution.master(tspan, tensor(ψ₀, dagger(ψ₀)), H_pump_tdop, J)
# Throws an ArgumentError with the message: "You are attempting to use a time-dependent dynamics generator (a Hamiltonian or Lindbladian) with a solver that assumes constant dynamics. To avoid errors, please use the _dynamic solvers instead, e.g. schroedinger_dynamic instead of schroedinger"

Isn't it better to use the same method for this problem?

a-eghrari commented 1 month ago

I made a mistake and closed this PR.

Krastanov commented 1 month ago

Usually it is easy to reopen a pull request but it says that you have deleted the originating repository. Feel free to simply open a new pull request if you have the bandwidth to do so.

And you are right, having better, more thorough _check_const, which itself sends better error messages, would be good. In some situations though, especially when there is a "missing method dispatch" type of error, we do not ever reach _check_const so probably both are necessary.