eddelbuettel / rcppziggurat

Rcpp bindings for different Ziggurat RNG implementations
Other
12 stars 4 forks source link

Fully initialize state when setting the seed (fixes #3) #7

Closed rstub closed 7 years ago

rstub commented 7 years ago

I would also like to extend the tests, but I am unsure how those work.

eddelbuettel commented 7 years ago

So two things.

First, are we really sure that the jsr = 123456789; assignment is needed? After all, it is set in all in the derived classes we actually instantiate --- inst/include/*. I don't yet see a use case where you need this (and I think #3 and the SO question are wrong).

Second, what do mean by "extend the tests" (ie add more?) and by "unsure how it works"? We use the tests/ directory with reference output as created by R CMD check. See eg "Writing R Extensions".

eddelbuettel commented 7 years ago

See for example a use of seeding with the current CRAN version of the package, ie without your patch:

edd@brad:~$ Rscript -e 'library(RcppZiggurat); zsetseed(456); print(zrnorm(2)); zsetseed(789); print(zrnorm(2))'
[1] -1.376230 -0.730005
[1]  0.1897669 -0.7272849
edd@brad:~$ Rscript -e 'library(RcppZiggurat); zsetseed(456); print(zrnorm(2)); zsetseed(789); print(zrnorm(2))'
[1] -1.376230 -0.730005
[1]  0.1897669 -0.7272849
edd@brad:~$

We can set a seed perfectly well, and reproduce the results.

/cc #3

rstub commented 7 years ago

From my point of view zsetseed() should set the RNG to a definite state irrespective of the history. The current implementation in Ziggurat.h uses the current state of jsr. This makes the RNG history dependent. Simple examples using the current CRAN version:

ralf@pc-ralf:~/devel$ Rscript -e 'library(RcppZiggurat); zsetseed(456); print(zrnorm(2)); zsetseed(789); print(zrnorm(2))'
[1] -1.376230 -0.730005
[1]  0.1897669 -0.7272849
ralf@pc-ralf:~/devel$ Rscript -e 'library(RcppZiggurat); zsetseed(789); print(zrnorm(2))'
[1] -1.1285849 -0.1585969
ralf@pc-ralf:~/devel$ Rscript -e 'library(RcppZiggurat); zsetseed(789); print(zrnorm(2)); zsetseed(789); print(zrnorm(2))'
[1] -1.1285849 -0.1585969
[1] 0.03028204 1.36236769

The first call reproduces your example. The second call shows that using the same seed '789' yields a different result if it is used as first seed. The third example shows that using the same seed multiple times within one R session gives different results (that was the original issue from SO). This is different from base functionality:

ralf@pc-ralf:~/devel$ Rscript -e 'set.seed(789); print(rnorm(2)); set.seed(789); print(rnorm(2))'
[1]  0.5240967 -2.2607679
[1]  0.5240967 -2.2607679

With the patch applied, I get reproducible random numbers that are independent of the RNG's history:

 ralf@pc-ralf:~/devel$ Rscript -e 'library(RcppZiggurat); zsetseed(456); print(zrnorm(2)); zsetseed(789); print(zrnorm(2))'
[1] -1.376230 -0.730005
[1] -1.1285849 -0.1585969
ralf@pc-ralf:~/devel$ Rscript -e 'library(RcppZiggurat); zsetseed(456); print(zrnorm(2)); zsetseed(789); print(zrnorm(2))'
[1] -1.376230 -0.730005
[1] -1.1285849 -0.1585969
ralf@pc-ralf:~/devel$ Rscript -e 'library(RcppZiggurat); zsetseed(789); print(zrnorm(2))'
[1] -1.1285849 -0.1585969
ralf@pc-ralf:~/devel$ Rscript -e 'library(RcppZiggurat); zsetseed(789); print(zrnorm(2)); zsetseed(789); print(zrnorm(2))'
[1] -1.1285849 -0.1585969
[1] -1.1285849 -0.1585969

I will extend the PR w.r.t zigguratTest.R.

eddelbuettel commented 7 years ago

Let me think about this:

The second call shows that using the same seed '789' yields a different result if it is used as first seed.

My gut reaction is that this is simply how Ziggurat works / is designed.

This is different from base functionality

Dittto

But your third example is good. I think I may have tried to be too-clever-by-half and have the assignment your PR brings already in the class so that it happens at instantiation. What I overlooked was the reset-during-session functionality your third examples shows. I am warming up to this. But still early-ish morning here :)

rstub commented 7 years ago

I am warming up to this. But still early-ish morning here :)

Take your time. :-) Just one more detail. I do not think that this is specific to the Ziggurat method, since ZigguratLZLLV behaves the way I am expecting (and also sets jsr in setSeed()):

ralf@pc-ralf:~/devel/rcppziggurat/inst/include$ Rscript -e 'library(RcppZiggurat); zsetseedLZLLV(456); print(zrnormLZLLV(2)); zsetseedLZLLV(789); print(zrnormLZLLV(2))'
[1] -1.376230 -0.730005
[1] -1.1285849 -0.1585969
ralf@pc-ralf:~/devel/rcppziggurat/inst/include$ Rscript -e 'library(RcppZiggurat); zsetseedLZLLV(456); print(zrnormLZLLV(2)); zsetseedLZLLV(789); print(zrnormLZLLV(2))'
[1] -1.376230 -0.730005
[1] -1.1285849 -0.1585969
ralf@pc-ralf:~/devel/rcppziggurat/inst/include$ Rscript -e 'library(RcppZiggurat); zsetseedLZLLV(789); print(zrnormLZLLV(2))'
[1] -1.1285849 -0.1585969
ralf@pc-ralf:~/devel/rcppziggurat/inst/include$ Rscript -e 'library(RcppZiggurat); zsetseedLZLLV(789); print(zrnormLZLLV(2)); zsetseedLZLLV(789); print(zrnormLZLLV(2))'
[1] -1.1285849 -0.1585969
[1] -1.1285849 -0.1585969