fix RNGState update in repeated_fisher_yates. Revise docstring for RN…

…GState. Replace some instances of r123::Philox4x32 with DefaultRNG.
BallisticLA · Sep 23, 2024 · a28b42d · a28b42d
1 parent 56aedb4
commit a28b42d
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 11 deletions.
diff --git a/RandBLAS/base.hh b/RandBLAS/base.hh
@@ -52,13 +52,19 @@ namespace RandBLAS {
 
 typedef r123::Philox4x32 DefaultRNG;
 
-/** A representation of the state of a counter-based random number generator
- * (CBRNG) defined in Random123. The representation consists of two arrays:
- * the counter and the key. The arrays' types are statically sized, small
- * (typically of length 2 or 4), and can be distinct from one another.
+/** 
+ * This type acts as a stateful version of a traditionally-stateless 
+ * counter-based random number generator (CBRNG) from Random123;
+ * it packages a CBRNG with a pair of arrays called "counter" and "key."
+ * 
+ * Most RandBLAS functions that accept an RNGState as an input will
+ * return a new RNGState with an appropriately updated counter.
+ * The only exceptions to this are constructors for SketchingOperator types,
+ * where the updated RNGState can be accessed as SketchingOperator::next_state.
+ * 
+ * Users can get access to independent "streams" of random numbers by
+ * defining CBRNGs with different keys; see RNGState constructors for more details.
  * 
- * The template parameter RNG is a CBRNG type in defined in Random123. We've found
- * that Philox-based CBRNGs work best for our purposes, but we also support Threefry-based CBRNGS.
  */
 template <typename RNG = DefaultRNG>
 struct RNGState {

diff --git a/RandBLAS/dense_skops.hh b/RandBLAS/dense_skops.hh
@@ -333,7 +333,7 @@ static_assert(SketchingDistribution<DenseDist>);
 ///  A sample from a distribution over matrices whose entries are iid
 ///  mean-zero variance-one random variables.
 ///  This type conforms to the SketchingOperator concept.
-template <typename T, typename RNG = r123::Philox4x32>
+template <typename T, typename RNG = DefaultRNG>
 struct DenseSkOp {
 
     // ---------------------------------------------------------------------------
@@ -537,7 +537,7 @@ static_assert(SketchingOperator<DenseSkOp<double>>);
 ///      - Used to define :math:`\mtxS` as a sample from :math:`\D.`
 ///
 /// @endverbatim
-template<typename T, typename RNG = r123::Philox4x32>
+template<typename T, typename RNG = DefaultRNG>
 RNGState<RNG> fill_dense_unpacked(blas::Layout layout, const DenseDist &D, int64_t n_rows, int64_t n_cols, int64_t ro_s, int64_t co_s, T* buff, const RNGState<RNG> &seed) {
     using RandBLAS::dense::fill_dense_submat_impl;
     randblas_require(D.n_rows >= n_rows + ro_s);
@@ -597,7 +597,7 @@ RNGState<RNG> fill_dense_unpacked(blas::Layout layout, const DenseDist &D, int64
 ///      A CBRNG state
 ///      - Used to define \math{\mat(\buff)} as a sample from \math{\D}.
 ///
-template <typename T, typename RNG = r123::Philox4x32>
+template <typename T, typename RNG = DefaultRNG>
 RNGState<RNG> fill_dense(const DenseDist &D, T *buff, const RNGState<RNG> &seed) {
     return fill_dense_unpacked(D.natural_layout, D, D.n_rows, D.n_cols, 0, 0, buff, seed);
 }

diff --git a/RandBLAS/sparse_skops.hh b/RandBLAS/sparse_skops.hh
@@ -101,6 +101,7 @@ static state_t repeated_fisher_yates(
             vec_work[ell] = swap;
         }
     }
+    ctr.incr(dim_minor * vec_nnz);
     return state_t {ctr, key};
 }
 
@@ -239,7 +240,7 @@ inline state_t repeated_fisher_yates(
     return sparse::repeated_fisher_yates(state, k, n, r, samples, (sint_t*) nullptr, (double*) nullptr);
 }
 
-template <typename RNG>
+template <typename RNG = DefaultRNG>
 RNGState<RNG> compute_next_state(SparseDist dist, RNGState<RNG> state) {
     int64_t num_mavec, incrs_per_mavec;
     if (dist.major_axis == Axis::Short) {
@@ -262,7 +263,7 @@ RNGState<RNG> compute_next_state(SparseDist dist, RNGState<RNG> state) {
 /// A sample from a distribution over structured sparse matrices with either
 /// independent rows or independent columns. This type conforms to the
 /// SketchingOperator concept.
-template <typename T, typename RNG = r123::Philox4x32, SignedInteger sint_t = int64_t>
+template <typename T, typename RNG = DefaultRNG, SignedInteger sint_t = int64_t>
 struct SparseSkOp {
 
     // ---------------------------------------------------------------------------