Skip to content
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

Adding hint for master solver function #388

Closed
wants to merge 1 commit into from
Closed

Adding hint for master solver function #388

wants to merge 1 commit into from

Conversation

a-eghrari
Copy link
Contributor

@a-eghrari a-eghrari commented May 1, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Contributor Author

@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 a-eghrari closed this by deleting the head repository May 12, 2024
@a-eghrari
Copy link
Contributor Author

I made a mistake and closed this PR.

@Krastanov
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants