Open frankschae opened 5 months ago
Hmm, I think I did this on purpose. In the first run, it uses the value provided by rv
. In the subsequent runs, it calls RV!
and gets a new value. If you want to start with a random value, just call RV!
early on or initialize rv
with a random value instead of zero or something else.
Right, I can also see this point -- and I read the docs again, and you wrote it already pretty explicit:
An optional realization rv may be given. The realization rv is used in the first time an AbstractRODEProblem is solved. Subsequent runs of the same problem will draw a different realization from the random variable RV, unless reseed is set to false
We should perhaps add this to the example with some additional comments as well? (That it's not just the rv "prototype" but really the rv for the first run.)
Now that you drew attention to this, I come to realize it might not have been a good design option.
The whole sentence is confusing:
An optional realization
rv
may be given. The realizationrv
is used in the first time anAbstractRODEProblem
is solved. Subsequent runs of the same problem will draw a different realization from the random variableRV
, unlessreseed
is set to false. In the case of aNoiseProblem
, however, a new realization will happen at the first run already, and, in this case,rv
can be regarded as a realization prototype, which is necessary in the case of a random vector.
It says it is optional, but at the end it says it is necessary in the case of a random vector.
At the moment, I think the best thing is to consider rv
as a prototype and always start with a new realization, and perhaps we add a keyword option to keep the given value of rv
as a starting value, if one wishes to do so. What do you think?
Addresses https://github.com/SciML/DiffEqNoiseProcess.jl/issues/197
The values of
v
are indeed zero for the first run of the SDE solver. This seems to be a bug. For later runs, they do get initialized byreinit!
https://github.com/SciML/StochasticDiffEq.jl/pull/502/files#diff-ca1c45d4f0bec64e8340981f6b27f4037bf6ac13c8f1af132656c485c0a8e941R422An easy fix (?) seems to be to force initialization in the constructor. This is already done for oop/scalar processes but not for inplace/array-like random variables (see changes below). Given that the docs say:
I think the fix below should be fine.
@rmsrosa could you have a look at this change as well, please? I haven't used
NoiseTransport
myself yet...and I am not fully confident I oversee all potential difficulties.Checklist
Additional context
Add any other context about the problem here.