dqrng icon indicating copy to clipboard operation
dqrng copied to clipboard

Failures from reverse dependency checks

Open rstub opened this issue 1 year ago • 40 comments

Reverse dependency checks via GHA are still not working, but running them locally gives me two failures:

GridOnClusters

  • Version: 0.1.0
  • GitHub: NA
  • Source code: https://github.com/cran/GridOnClusters
  • Date/Publication: 2022-01-28 05:40:02 UTC
  • Number of recursive dependencies: 65

Run revdepcheck::revdep_details(, "GridOnClusters") for more info

Newly broken

  • checking tests ...
      Running ‘testthat.R’
     ERROR
    Running the tests in ‘tests/testthat.R’ failed.
    Last 13 lines of output:
      Mean relative difference: 0.192
      ── Failure ('test-GridOnClusters.R:200:3'): Testing discretize.jointly ("kmeans+silhouette") ──
      `dim13` not equivalent to as.table(...).
      Mean relative difference: 0.182
      ── Failure ('test-GridOnClusters.R:208:3'): Testing discretize.jointly ("kmeans+silhouette") ──
      `dim23` not equivalent to as.table(...).
      Mean relative difference: 0.072
      ── Failure ('test-GridOnClusters.R:214:3'): Testing discretize.jointly ("kmeans+silhouette") ──
      round(discr$csimilarity, digits = 3) not equivalent to 0.915.
      1/1 mismatches
      [1] 0.849 - 0.915 == -0.066
    
      [ FAIL 30 | WARN 0 | SKIP 0 | PASS 10 ]
      Error: Test failures
      Execution halted
    

@joemsong: Could this be related to the change in default RNG? If I find an explanation: Do you have a way to accept a PR?

rnndescent

  • Version: 0.1.4
  • GitHub: https://github.com/jlmelville/rnndescent
  • Source code: https://github.com/cran/rnndescent
  • Date/Publication: 2024-03-18 05:40:02 UTC
  • Number of recursive dependencies: 62

Run revdepcheck::revdep_details(, "rnndescent") for more info

Newly broken

  • checking whether package ‘rnndescent’ can be installed ... ERROR
    Installation failed.
    See ‘/home/ralf/devel/dqrng/revdep/checks/rnndescent/new/rnndescent.Rcheck/00install.out’ for details.
    

Newly fixed

  • checking installed package size ... NOTE
      installed size is 21.7Mb
      sub-directories of 1Mb or more:
        libs  20.6Mb
    

Installation

Devel

* installing *source* package ‘rnndescent’ ...
** package ‘rnndescent’ successfully unpacked and MD5 sums checked
** using staged installation
** libs
using C++ compiler: ‘g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0’
using C++17
g++ -std=gnu++17 -I"/opt/R/4.3.0/lib/R/include" -DNDEBUG -I../inst/include/ -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/BH/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/dqrng/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/Rcpp/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/sitmo/include' -I/usr/local/include   -DSTRICT_R_HEADERS -DRCPP_NO_MODULES -fpic  -g -O2  -c RcppExports.cpp -o RcppExports.o
g++ -std=gnu++17 -I"/opt/R/4.3.0/lib/R/include" -DNDEBUG -I../inst/include/ -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/BH/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/dqrng/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/Rcpp/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/sitmo/include' -I/usr/local/include   -DSTRICT_R_HEADERS -DRCPP_NO_MODULES -fpic  -g -O2  -c rnn_bruteforce.cpp -o rnn_bruteforce.o
g++ -std=gnu++17 -I"/opt/R/4.3.0/lib/R/include" -DNDEBUG -I../inst/include/ -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/BH/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/dqrng/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/Rcpp/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/sitmo/include' -I/usr/local/include   -DSTRICT_R_HEADERS -DRCPP_NO_MODULES -fpic  -g -O2  -c rnn_hub.cpp -o rnn_hub.o
g++ -std=gnu++17 -I"/opt/R/4.3.0/lib/R/include" -DNDEBUG -I../inst/include/ -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/BH/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/dqrng/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/Rcpp/include' -I'/home/ralf/devel/dqrng/revdep/library/dqrng/new/sitmo/include' -I/usr/local/include   -DSTRICT_R_HEADERS -DRCPP_NO_MODULES -fpic  -g -O2  -c rnn_indextograph.cpp -o rnn_indextograph.o
...
In file included from rnn_nnd.cpp:24:
../inst/include/rnndescent/random.h: In function ‘dqrng::rng64_t rnndescent::create_dqrng()’:
../inst/include/rnndescent/random.h:49:62: error: could not convert ‘std::make_shared(_Args&& ...) [with _Tp = dqrng::random_64bit_wrapper<pcg_detail::engine<long unsigned int, __int128 unsigned, pcg_detail::xsl_rr_mixin<long unsigned int, __int128 unsigned>, false, pcg_detail::specific_stream<__int128 unsigned>, pcg_detail::default_multiplier<__int128 unsigned> > >; _Args = {}]()’ from ‘std::shared_ptr<dqrng::random_64bit_wrapper<pcg_detail::engine<long unsigned int, __int128 unsigned, pcg_detail::xsl_rr_mixin<long unsigned int, __int128 unsigned>, false, pcg_detail::specific_stream<__int128 unsigned>, pcg_detail::default_multiplier<__int128 unsigned> > > >’ to ‘dqrng::rng64_t’ {aka ‘Rcpp::XPtr<dqrng::random_64bit_generator>’}
   49 |   return std::make_shared<dqrng::random_64bit_wrapper<pcg64>>();
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
      |                                                              |
      |                                                              std::shared_ptr<dqrng::random_64bit_wrapper<pcg_detail::engine<long unsigned int, __int128 unsigned, pcg_detail::xsl_rr_mixin<long unsigned int, __int128 unsigned>, false, pcg_detail::specific_stream<__int128 unsigned>, pcg_detail::default_multiplier<__int128 unsigned> > > >
make: *** [/opt/R/4.3.0/lib/R/etc/Makeconf:200: rnn_nnd.o] Error 1
ERROR: compilation failed for package ‘rnndescent’
* removing ‘/home/ralf/devel/dqrng/revdep/checks/rnndescent/new/rnndescent.Rcheck/rnndescent’

@jlmelville This is clearly my redefinition of dqrng::rng64_t. It might make sense to use dqrng::generator<pcg64>() instead. I can provide a PR for that. BTW, given that I have factored out the sampling code into dqrng_sample.h: Do you still need your own copy dqsample.h?

rstub avatar Apr 10 '24 08:04 rstub

@rstub thanks for the heads-up, I'll take a look at this as soon as I can

jlmelville avatar Apr 10 '24 14:04 jlmelville

@jlmelville Great! The following patch works for me both with the CRAN version and the version from #79:

diff --git a/inst/include/rnndescent/random.h b/inst/include/rnndescent/random.h
index 9619b37..09b0b3c 100644
--- a/inst/include/rnndescent/random.h
+++ b/inst/include/rnndescent/random.h
@@ -46,7 +46,13 @@ inline auto r_seed() -> uint64_t {
 }
 
 inline auto create_dqrng() -> dqrng::rng64_t {
-  return std::make_shared<dqrng::random_64bit_wrapper<pcg64>>();
+  auto seed1 = r_seed();
+  auto seed2 = r_seed();
+  return dqrng::generator<pcg64>(seed1, seed2);
+}
+
+inline auto create_dqrng(uint64_t seed, uint64_t seed2) -> dqrng::rng64_t {
+  return dqrng::generator<pcg64>(seed, seed2);
 }
 
 inline auto combine_seeds(uint32_t msw, uint32_t lsw) -> uint64_t {
@@ -65,8 +71,7 @@ struct TauRand : public tdoann::RandomGenerator {
   std::unique_ptr<tdoann::tau_prng> prng{nullptr};
 
   TauRand(uint64_t seed, uint64_t seed2) {
-    dqrng::rng64_t rng = create_dqrng();
-    rng->seed(seed, seed2);
+    dqrng::rng64_t rng = create_dqrng(seed, seed2);
 
     // Stitch together 3 64-bit ints from 6 32-bit ones
     std::vector<uint32_t> tau_seeds32;
@@ -114,15 +119,9 @@ private:
 public:
 
   // Not thread safe
-  DQIntSampler() : rng(create_dqrng()) {
-    auto seed1 = r_seed();
-    auto seed2 = r_seed();
-    rng->seed(seed1, seed2);
-  }
+  DQIntSampler() : rng(create_dqrng()) {}
 
-  DQIntSampler(uint64_t seed, uint64_t seed2) : rng(create_dqrng()) {
-    rng->seed(seed, seed2);
-  }
+  DQIntSampler(uint64_t seed, uint64_t seed2) : rng(create_dqrng(seed, seed2)) {}
 
   // Generates a random integer in range [0, n)
   Int rand_int(Int n) override {

Feel free to use it. I can also provide a PR for it.

rstub avatar Apr 12 '24 21:04 rstub

More packages seem to be affected. It looks like those are affected by the change in default RNG. Most likely it would help to add dqRNGkind("xoroshiro128+") before dqset.seed(...) in the test code:

FunChisq

@joemsong

* checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  round(xtoy$statistic, digits = 2) not equivalent to 53.48.
  1/1 mismatches
  [1] 55 - 53.5 == 1.5
  ── Failure ('test_AdpFunChisq.R:65:3'): Testing the adapted functional chi-squared test ──
  signif(xtoy$p.value, digits = 2) not equivalent to 0.81.
  1/1 mismatches
  [1] 0.87 - 0.81 == 0.06
  ── Failure ('test_AdpFunChisq.R:66:3'): Testing the adapted functional chi-squared test ──
  round(xtoy$statistic, digits = 2) not equivalent to 3.
  1/1 mismatches
  [1] 2.51 - 3 == -0.49
  
  [ FAIL 5 | WARN 0 | SKIP 0 | PASS 242 ]
  Error: Test failures
  Execution halted

fwildclusterboot

@s3alfisc

* checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
    'test-r-vs-julia.R:4:3', 'test-seed.R:3:3', 'test-tstat_equivalence.R:335:3',
    'test-tstat_equivalence.R:561:3', 'test-tstat_equivalence.R:840:3',
    'test-uncategorized.R:3:3', 'test_sunab.R:3:3', 'test_tidy.R:3:3',
    'test_tidy.R:64:3'
  
  ══ Failed tests ════════════════════════════════════════════════════════════════
  ── Failure ('test_samplers.R:36:3'): test sampling ─────────────────────────────
  confint(boot1) (`actual`) not equal to confint(boot2) (`expected`).
  
    `actual`: -0.015 -0.000
  `expected`: -0.016 -0.000
  
  [ FAIL 1 | WARN 1 | SKIP 19 | PASS 180 ]
  Error: Test failures
  Execution halted

greeks

@ahudde

* checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  > library(greeks)
  > 
  > test_check("greeks")
  [1] "custom payoff"
  [1] "custom payoff"
  [1] "custom payoff"
  [ FAIL 1 | WARN 8 | SKIP 0 | PASS 20 ]
  
  ══ Failed tests ════════════════════════════════════════════════════════════════
  ── Failure ('test-Malliavin_European_Greeks.R:59:3'): Malliavin_European_Greeks is correct ──
  unknown failure
  
  [ FAIL 1 | WARN 8 | SKIP 0 | PASS 20 ]
  Error: Test failures
  Execution halted

rstub avatar Apr 12 '24 21:04 rstub

@rstub is the current state of main the correct code for testing? When I use that your patch it does fix the compilation error, but I am now getting linker errors later:

   g++ -shared -static-libgcc -o rnndescent.dll tmp.def RcppExports.o rnn_bruteforce.o rnn_hub.o rnn_indextograph.o rnn_merge.o rnn_nnd.o rnn_prepare.o rnn_randnbrs.o rnn_rptree.o rnn_search.o rnn_util.o -LC:/rtools43/x86_64-w64-mingw32.static.posix/lib/x64 -LC:/rtools43/x86_64-w64-mingw32.static.posix/lib -LC:/PROGRA~1/R/R-43~1.3/bin/x64 -lR
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_prepare.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:664: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_prepare.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:664: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_randnbrs.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_randnbrs.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_rptree.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/dqrng/include/dqrng.h:25: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_rptree.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/dqrng/include/dqrng.h:25: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_search.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:664: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: rnn_search.o: in function `dqrng::random_64bit_accessor::random_64bit_accessor()':
   E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:664: multiple definition of `dqrng::random_64bit_accessor::random_64bit_accessor()'; rnn_nnd.o:E:/dev/R/win-library/4.0/Rcpp/include/Rcpp/utils/tinyformat/tinyformat.h:674: first defined here
   collect2.exe: error: ld returned 1 exit status
   no DLL was created
   ERROR: compilation failed for package 'rnndescent'

Also: yes I would be happy to see if I can stop using dqsample.h once I can build again.

jlmelville avatar Apr 13 '24 18:04 jlmelville

@jlmelville Please use the state from #79 for testing. Current main is missing an important inline.

Concering dqsample.h: It is best to wait until I have done a CRAN release since dqrng_sample.h will change in that release. At the moment, it expects dqrng::rng64_t as argument and will change to dqrng::random_64bit_generator&. So it is important to have a version on CRAN that works both the CRAN of dqrng and the version from #79.

rstub avatar Apr 13 '24 18:04 rstub

@jlmelville Please use the state from #79 for testing. Current main is missing an important inline.

Got it. It's working now on #79 and the CRAN release of dqrng. In terms of coordinating, do you want me to prep a new release of rnndescent to CRAN? Let me know when it would be ok to submit that.

Thank you for the patch. Do you object strongly to being listed as a contributor in rnndescent?

Concering dqsample.h: It is best to wait until I have done a CRAN release since dqrng_sample.h will change in that release. At the moment, it expects dqrng::rng64_t as argument and will change to dqrng::random_64bit_generator&.

Understood.

jlmelville avatar Apr 13 '24 19:04 jlmelville

@jlmelville You could upload anytime, but it probably makes sense to wait until I have finalized #79. Feel free to add me as contributor if you see this as important enough.

rstub avatar Apr 13 '24 22:04 rstub

@s3alfisc: I was able to reproduce the issue with the CRAN version 0.13 of fwildclusterboot, while both the current development version as well as version 0.14.3 from r-universe does not show this error. Do you have any plans for a CRAN release? BTW, a minimal fix for v0.13 would be to insert dqrng::dgRNGkind("xoroshiro128+") in test_samplers.R.

rstub avatar Apr 15 '24 13:04 rstub

@joemsong: I was able to reproduce the issue with the CRAN versions of both GridOnClusters and FunChisq. In both cases, it is enough to insert dqrng::dgRNGkind("xoroshiro128+") in the relevant test file. Are you interested in me providing a patch?

rstub avatar Apr 15 '24 21:04 rstub

@ahudde: I was able to reproduce the issue with the CRAN version of greeks. It is enough to insert dqrng::dgRNGkind("xoroshiro128+") in the relevant test file. However, in the current development version I see a different failure which I cannot fix this way.

rstub avatar Apr 15 '24 21:04 rstub

Do you have any plans for a CRAN release?

Hi @rstub , I have no immediate plans for a CRAN release - would you like me to submit a hotfix to 0.13 by adding dqrng::dgRNGkind("xoroshiro128+")? Then I would tackle this tomorrow after work. Sorry for taking a few days to respond, I was traveling over the weekend.

s3alfisc avatar Apr 16 '24 19:04 s3alfisc

Dear @rstub, Sorry for the inconvenience. And thank you very much for your great packages, which really helps improving performance of my package. I hope I can find the time to look into it the coming weekend.

ahudde avatar Apr 16 '24 19:04 ahudde

Thanks @ahudde and @s3alfisc for looking into this! A CRAN upload of your packages in the coming weeks with an appropriate fix would be great.

rstub avatar Apr 17 '24 07:04 rstub

@jlmelville You could upload anytime, but it probably makes sense to wait until I have finalized #79. Feel free to add me as contributor if you see this as important enough.

Just to confirm @rstub, as #79 is complete, would it now be helpful to submit a new version of rnndescent?

jlmelville avatar Apr 18 '24 05:04 jlmelville

@jlmelville Yes, please go ahead.

rstub avatar Apr 18 '24 09:04 rstub

@jlmelville Yes, please go ahead.

A new version of rnndescent (0.1.5) is now on CRAN. Hopefully it passes all the usual checks. If you are still doing reverse dependency checks it might be worth trying on the new version of rnndescent, just in case.

jlmelville avatar Apr 19 '24 05:04 jlmelville

:tada: Latest revdep check no longer shows a failure with rnndescent. Interestingly, fwildclusterboot was fine as well, even though this is still the original version.

rstub avatar Apr 19 '24 09:04 rstub

@s3alfisc: It is odd, that fwildclusterboot does not show any problems in the last revdep check. And looking at the previous results it was actually the CRAN version of dqrng that produced the error:

Package: fwildclusterboot
Check: tests
Old result: ERROR
New result: OK 

Looking at the test file in question at https://github.com/s3alfisc/fwildclusterboot/blob/master/tests/testthat/test_samplers.R it looks like you are not setting any seeds. Might this be the reason? Setting a fixed RNG would still make sense, though.

rstub avatar Apr 19 '24 11:04 rstub

Thanks @ahudde and @s3alfisc for looking into this! A CRAN upload of your packages in the coming weeks with an appropriate fix would be great.

Dear @rstub, a new greeks-Version where the tests work with the new dqrng-Version is now CRAN.

ahudde avatar Apr 24 '24 10:04 ahudde

@jlmelville This is strange. I am currently seeing reverse dependency failures from rnndescent again. This time not from the installation but from the tests in test_rptree.R. Do you know what you are doing differently in that file compared with other tests? As for things that have changed: The version from #79 already changed the way the two argument ctor for PCG64 is used. I changed that again in #82, but I would have expected that code that works with CRAN version and the version from #79 to also work with the version from #82, i.e. results might change slightly but the overall result should be the same. Any idea whay is going on?

rstub avatar Apr 29 '24 11:04 rstub

@rstub by building off the current main (I see #82 has been merged so I think this the right thing to test?), I can reproduce the test failures.

The tree tests are very sensitive to the results of the random number generator. So if for a given seed the random numbers generated by dqrng change, so will those results. If that's what's happened with #82, I will need to update those tests to match the new output. So that wouldn't be an unexpected failure.

But if the stream of random numbers wasn't supposed to change then I will need to do some debugging to generate the stream of random numbers before and after.

jlmelville avatar May 04 '24 16:05 jlmelville

@jlmelville This is very odd. The results from the RNG for your use-case have changed multiple times, since I changed how the two argument construction dqrng::generator<pcg64>(seed, stream) works. AFAIK this is the only way how you are making use of RNGs from {dqrng}.

  1. The CRAN version 0.3.2 uses the two argument ctor pcg64(seed, stream) for this. You test cases are working with this version.

  2. In #79 (merged in 6819b9a75ba95b2349efe515a3a42919bfcd35f5 as version 0.3.2.4) I changed that to use pcg64(seed) plus pcg64::set_stream(stream) in order to get the same results from the PCG64 example in the parallel vignette when re-writing it while accessing the global RNG. See also https://github.com/imneme/pcg-cpp/issues/91. This should have changed the stream of random numbers you are seeing in your tests, but your test cases are still working.

  3. In #82 (merged in f2492c5cd961fc4b4669e9051ddd3846d973f3be as version 0.3.2.6) I realized that the example was not working as expected. Instead we developed the parallel_generate template and there it was necessary for clone(0) to return an unchanged RNG, resulting in another change of how the two argument construction is done. Note, one should use current main or at least #84 (merged in 2a188947a8d941e5929a020a87e7a68d05e6b47a) since an important inline was missing before that. This should have changed the stream of random numbers you are seeing in your tests, and now your test cases are no-longer working.

  4. In #87 (not yet merged, version 0.3.2.7) I thought it should be possible to bring back the original interpretation of the two argument construction. This should have changed the stream of random numbers you are seeing in your tests back to the CRAN version, however, your test cases are still not working.

I am surprised by the success in 2. and especially by the failure in 4.

rstub avatar May 06 '24 20:05 rstub

@rstub that is very perturbing. If I had to guess I would say I am doing something accidentally terrible like indirectly calling the R API from a thread or some kind of memory issue where the RNG state is being trampled on. I do test with valgrind/ABSAN/USAN but easy for something to slip through the cracks. I'll look more closely at that part of the code.

jlmelville avatar May 09 '24 05:05 jlmelville

@joemsong: Thanks for the update to both GridOnClusters and FunChisq. Reverse dependency checks appear good again.

rstub avatar May 11 '24 14:05 rstub

@jlmelville I have found the issue. In https://github.com/daqana/dqrng/pull/87 I had made sure that random_64bit_wrapper<pcg64>::seed(seed, stream) was unchanged, but not dqrng::generator<pcg64>(seed, stream). The former is used by R, why you are using the latter. I have now fixed that, so that now the reverse dependency check for rnndescent appear good again.

rstub avatar May 11 '24 14:05 rstub

Good news @rstub. Is there a branch/PR I can use to test on my side too?

jlmelville avatar May 11 '24 15:05 jlmelville

@jlmelville #87 contains the current state of things. Thanks for testing!

rstub avatar May 11 '24 15:05 rstub

@rstub I have tested with #87 and I can confirm tests are back to passing. So should be good to go from me. Thank you!

jlmelville avatar May 11 '24 22:05 jlmelville

The most recent reverse dependency check looks good. I am closing this ticket and going to upload to package. Thanks everybody!

@s3alfisc fwildclusterboot is still strange. Sometimes it fails with the CRAN version, sometimes with the new version, sometimes with both (like in the last run), and sometimes not at all. Let's hope it works on CRAN like it is working there right now.

rstub avatar May 12 '24 20:05 rstub

@rstub I hate to be the bearer of... confusing news, but I was working on some bug fixes on rnndescent last night (not related to anything to do with random number generators) and added some tests to the master branch a few hours ago.

Unfortunately, when I run R CMD check on my master branch with #87 (and now the main branch of dqrng) I get a segfault during the test run, which I do not see with the CRAN release of dqrng.

I am not currently sure what is going on. I have reproduced the problem on Windows and Mac -- those are on R 4.4.0. On Linux (stuck on 4.3.1) I can't reproduce it (valgrind did not show me anything either). Even on Windows and Mac, it does not appear when I run devtools::test, only devtools::check -- but it does appear when running R CMD check from the command line on a source tarball so I can't discount it as some devtools/RStudio weirdness.

jlmelville avatar May 12 '24 20:05 jlmelville