Closed jebej closed 1 year ago
I am not familiar with the method, but browsing the discussions, what about, instead of removing c2
, adding another keyword argument to BAOAB()
with the option of changing the default behavior?
I just suggested that same thing in another way. I agree it should just have two kwargs.
If we want another keyword argument, the only thing I can think of is something like scale_noise=true
, which keeps the current behavior, and when false just sets the c2
variable to 1. Anything else can be handled in the user function.
Or if it's another float, you just make the default c2 = gamma
.
Either way is fine I think, but I think the main point is that it needs two args not one.
If we want the original behavior from the paper we need dt
:
c1 = exp(-alg.gamma*dt)
c2 = sqrt(1 - c1^2)
It's not already scaling by dt?
From https://doi.org/10.1093/amrx/abs010 (c2 and c3 were switched)
As discussed in #519, I want to experiment with removing the
gamma
dependence attached to the noise term for theBAOAB
algorithm.The main issue is that if you do want the
gamma
dependence, you also need a dependence ondt
(coming from thec1
variable in the original code). While thisdt
dependence could be user-coded in theg
noise function, it's not an ideal interface.Suggestions welcome.
EDIT: instead of removing the scaling wholesale, there is now a
scale_noise::Bool
argument toBAOAB
.