-
Notifications
You must be signed in to change notification settings - Fork 1
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
Insert of coupling_rescale in wide_band_bath_discretisation.jl #42
base: main
Are you sure you want to change the base?
Conversation
Introduction of the keyword 'coupling_rescale' in 'fillbathcouplings!' and 'setcoupling!'
Introduction of 'coupling_rescale' keyword in the struct 'AndersonHolstein' as well as optional keyword in the function AndersonHolstein with the default value 1.0
Introduction of the keyword 'coupling_rescale' into fillbathcouplings!
@@ -8,19 +8,19 @@ function fillbathstates!(out::Hermitian, bath::WideBandBathDiscretisation) | |||
copy!(diagonal, bath.bathstates) | |||
end | |||
|
|||
function fillbathcoupling!(out::Hermitian, coupling::Real, bath::WideBandBathDiscretisation) | |||
function fillbathcoupling!(out::Hermitian, coupling::Real, bath::WideBandBathDiscretisation, coupling_rescale) |
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.
do you want to have coupling_rescale in fillbathcoupling!
and setcoupling!
as an optional keyword? there might be some use cases where these functions might be needed without.
also, we will probably at some point need multiple despatch for these as we will have scenarios where the coupling isn't a global real constant. Therefore, I recommend specifically setting the type to Real already here and in the other similar functions.
|
||
NQCModels.DiabaticModels.fillbathstates!(out, bath) | ||
@test all(out[diagind(out)[2:end]] .== bath.bathstates) | ||
NQCModels.DiabaticModels.fillbathcoupling!(out, 1.0, bath) | ||
NQCModels.DiabaticModels.fillbathcoupling!(out, 1.0, bath, coupling_rescale) |
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.
now you are only testing if the functions run WITH the optional keyword set. you should also have a test where this isn't set.
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.
this looks great. Have a look at the comments I left in the files
Introduction of the keyword 'coupling_rescale' in 'fillbathcouplings!', 'setcoupling!' and in NewnsAnderson models. This allows the coupling in a 2x2 'model' to be rescaled such that the discretized NAH has the same ground state energy landscape as the 2x2 'model'.
For now, this optional coupling_rescale parameter will be a global scalar, but in the future, we will change this to a coordinate-dependent scaling function.