diff --git a/nalgebra-sparse/src/proptest.rs b/nalgebra-sparse/src/proptest.rs index 1115693a..757c539b 100644 --- a/nalgebra-sparse/src/proptest.rs +++ b/nalgebra-sparse/src/proptest.rs @@ -2,6 +2,10 @@ //! //! TODO: Clarify that this module needs proptest-support feature +// Contains some patched code from proptest that we can remove in the (hopefully near) future. +// See docs in file for more details. +mod proptest_patched; + use crate::coo::CooMatrix; use proptest::prelude::*; use proptest::collection::{vec, hash_map, btree_set}; @@ -16,12 +20,20 @@ use crate::csc::CscMatrix; fn dense_row_major_coord_strategy(nrows: usize, ncols: usize, nnz: usize) -> impl Strategy> { + assert!(nnz <= nrows * ncols); let mut booleans = vec![true; nnz]; booleans.append(&mut vec![false; (nrows * ncols) - nnz]); // Make sure that exactly `nnz` of the booleans are true - Just(booleans) - // Need to shuffle to make sure they are randomly distributed - .prop_shuffle() + + // TODO: We cannot use the below code because of a bug in proptest, see + // https://github.com/AltSysrq/proptest/pull/217 + // so for now we're using a patched version of the Shuffle adapter + // (see also docs in `proptest_patched` + // Just(booleans) + // // Need to shuffle to make sure they are randomly distributed + // .prop_shuffle() + + proptest_patched::Shuffle(Just(booleans)) .prop_map(move |booleans| { booleans .into_iter() @@ -265,8 +277,8 @@ pub fn sparsity_pattern( // If the required number of nonzeros is sufficiently dense, // we instead use a dense sampling dense_row_major_coord_strategy(nmajor, nminor, nnz) - .prop_map(move |triplets| { - let coords = triplets.into_iter(); + .prop_map(move |coords| { + let coords = coords.into_iter(); sparsity_pattern_from_row_major_coords(nmajor, nminor, coords) }).boxed() } diff --git a/nalgebra-sparse/src/proptest/proptest_patched.rs b/nalgebra-sparse/src/proptest/proptest_patched.rs new file mode 100644 index 00000000..fa3d9d25 --- /dev/null +++ b/nalgebra-sparse/src/proptest/proptest_patched.rs @@ -0,0 +1,146 @@ +//! Contains a modified implementation of `proptest::strategy::Shuffle`. +//! +//! The current implementation in `proptest` does not generate all permutations, which is +//! problematic for our proptest generators. The issue has been fixed in +//! https://github.com/AltSysrq/proptest/pull/217 +//! but it has yet to be merged and released. As soon as this fix makes it into a new release, +//! the modified code here can be removed. +//! +/*! + This code has been copied and adapted from + https://github.com/AltSysrq/proptest/blob/master/proptest/src/strategy/shuffle.rs + The original licensing text is: + + //- + // Copyright 2017 Jason Lingle + // + // Licensed under the Apache License, Version 2.0 or the MIT license + // , at your + // option. This file may not be copied, modified, or distributed + // except according to those terms. + +*/ + +use proptest::strategy::{Strategy, Shuffleable, NewTree, ValueTree}; +use proptest::test_runner::{TestRunner, TestRng}; +use std::cell::Cell; +use proptest::num; +use proptest::prelude::Rng; + +#[derive(Clone, Debug)] +#[must_use = "strategies do nothing unless used"] +pub struct Shuffle(pub(super) S); + +impl Strategy for Shuffle + where + S::Value: Shuffleable, +{ + type Tree = ShuffleValueTree; + type Value = S::Value; + + fn new_tree(&self, runner: &mut TestRunner) -> NewTree { + let rng = runner.new_rng(); + + self.0.new_tree(runner).map(|inner| ShuffleValueTree { + inner, + rng, + dist: Cell::new(None), + simplifying_inner: false, + }) + } +} + +#[derive(Clone, Debug)] +pub struct ShuffleValueTree { + inner: V, + rng: TestRng, + dist: Cell>, + simplifying_inner: bool, +} + +impl ShuffleValueTree + where + V::Value: Shuffleable, +{ + fn init_dist(&self, dflt: usize) -> usize { + if self.dist.get().is_none() { + self.dist.set(Some(num::usize::BinarySearch::new(dflt))); + } + + self.dist.get().unwrap().current() + } + + fn force_init_dist(&self) { + if self.dist.get().is_none() { + let _ = self.init_dist(self.current().shuffle_len()); + } + } +} + +impl ValueTree for ShuffleValueTree + where + V::Value: Shuffleable, +{ + type Value = V::Value; + + fn current(&self) -> V::Value { + let mut value = self.inner.current(); + let len = value.shuffle_len(); + // The maximum distance to swap elements. This could be larger than + // `value` if `value` has reduced size during shrinking; that's OK, + // since we only use this to filter swaps. + let max_swap = self.init_dist(len); + + // If empty collection or all swaps will be filtered out, there's + // nothing to shuffle. + if 0 == len || 0 == max_swap { + return value; + } + + let mut rng = self.rng.clone(); + + for start_index in 0..len - 1 { + // Determine the other index to be swapped, then skip the swap if + // it is too far. This ordering is critical, as it ensures that we + // generate the same sequence of random numbers every time. + + // NOTE: The below line is the whole reason for the existence of this adapted code + // We need to be able to swap with the same element, so that some elements remain in + // place rather being swapped + // let end_index = rng.gen_range(start_index + 1, len); + let end_index = rng.gen_range(start_index, len); + if end_index - start_index <= max_swap { + value.shuffle_swap(start_index, end_index); + } + } + + value + } + + fn simplify(&mut self) -> bool { + if self.simplifying_inner { + self.inner.simplify() + } else { + // Ensure that we've initialised `dist` to *something* to give + // consistent non-panicking behaviour even if called in an + // unexpected sequence. + self.force_init_dist(); + if self.dist.get_mut().as_mut().unwrap().simplify() { + true + } else { + self.simplifying_inner = true; + self.inner.simplify() + } + } + } + + fn complicate(&mut self) -> bool { + if self.simplifying_inner { + self.inner.complicate() + } else { + self.force_init_dist(); + self.dist.get_mut().as_mut().unwrap().complicate() + } + } +} \ No newline at end of file diff --git a/nalgebra-sparse/tests/unit_tests/proptest.rs b/nalgebra-sparse/tests/unit_tests/proptest.rs index 8e5731e0..46bc5130 100644 --- a/nalgebra-sparse/tests/unit_tests/proptest.rs +++ b/nalgebra-sparse/tests/unit_tests/proptest.rs @@ -6,7 +6,7 @@ fn coo_no_duplicates_generates_admissible_matrices() { #[cfg(feature = "slow-tests")] mod slow { - use nalgebra_sparse::proptest::{coo_with_duplicates, coo_no_duplicates}; + use nalgebra_sparse::proptest::{coo_with_duplicates, coo_no_duplicates, csr, csc, sparsity_pattern}; use nalgebra::DMatrix; use proptest::test_runner::TestRunner; @@ -18,6 +18,7 @@ mod slow { use std::collections::HashSet; use std::iter::repeat; use std::ops::RangeInclusive; + use nalgebra_sparse::csr::CsrMatrix; fn generate_all_possible_matrices(value_range: RangeInclusive, rows_range: RangeInclusive, @@ -73,19 +74,15 @@ mod slow { let values = -1..=1; let rows = 0..=2; let cols = 0..=3; - let strategy = coo_no_duplicates(values.clone(), rows.clone(), cols.clone(), 2 * 3); + let max_nnz = rows.end() * cols.end(); + let strategy = coo_no_duplicates(values.clone(), rows.clone(), cols.clone(), max_nnz); // Enumerate all possible combinations let all_combinations = generate_all_possible_matrices(values, rows, cols); - let mut visited_combinations = HashSet::new(); - for _ in 0..num_generated_matrices { - let tree = strategy - .new_tree(&mut runner) - .expect("Tree generation should not fail"); - let matrix = tree.current(); - visited_combinations.insert(DMatrix::from(&matrix)); - } + let visited_combinations = sample_matrix_output_space(strategy, + &mut runner, + num_generated_matrices); assert_eq!(visited_combinations.len(), all_combinations.len()); assert_eq!(visited_combinations, all_combinations, "Did not sample all possible values."); @@ -108,21 +105,17 @@ mod slow { let values = -1..=1; let rows = 0..=2; let cols = 0..=3; - let strategy = coo_with_duplicates(values.clone(), rows.clone(), cols.clone(), 2 * 3, 2); + let max_nnz = rows.end() * cols.end(); + let strategy = coo_with_duplicates(values.clone(), rows.clone(), cols.clone(), max_nnz, 2); // Enumerate all possible combinations that fit the constraints // (note: this is only a subset of the matrices that can be generated by // `coo_with_duplicates`) let all_combinations = generate_all_possible_matrices(values, rows, cols); - let mut visited_combinations = HashSet::new(); - for _ in 0..num_generated_matrices { - let tree = strategy - .new_tree(&mut runner) - .expect("Tree generation should not fail"); - let matrix = tree.current(); - visited_combinations.insert(DMatrix::from(&matrix)); - } + let visited_combinations = sample_matrix_output_space(strategy, + &mut runner, + num_generated_matrices); // Here we cannot verify that the set of visited combinations is *equal* to // all possible outcomes with the given constraints, however the @@ -131,6 +124,110 @@ mod slow { // is contained in the set of visited matrices assert!(all_combinations.is_subset(&visited_combinations)); } -} -// TODO: Tests for csr, csc and sparsity_pattern strategies \ No newline at end of file + #[cfg(feature = "slow-tests")] + #[test] + fn csr_samples_all_admissible_outputs() { + // We use a deterministic test runner to make the test "stable". + let mut runner = TestRunner::deterministic(); + + // This number needs to be high enough so that we with high probability sample + // all possible cases + let num_generated_matrices = 500000; + + let values = -1..=1; + let rows = 0..=2; + let cols = 0..=3; + let max_nnz = rows.end() * cols.end(); + let strategy = csr(values.clone(), rows.clone(), cols.clone(), max_nnz); + + let all_combinations = generate_all_possible_matrices(values, rows, cols); + + let visited_combinations = sample_matrix_output_space(strategy, + &mut runner, + num_generated_matrices); + + assert_eq!(visited_combinations.len(), all_combinations.len()); + assert_eq!(visited_combinations, all_combinations, "Did not sample all possible values."); + } + + #[cfg(feature = "slow-tests")] + #[test] + fn csc_samples_all_admissible_outputs() { + // We use a deterministic test runner to make the test "stable". + let mut runner = TestRunner::deterministic(); + + // This number needs to be high enough so that we with high probability sample + // all possible cases + let num_generated_matrices = 500000; + + let values = -1..=1; + let rows = 0..=2; + let cols = 0..=3; + let max_nnz = rows.end() * cols.end(); + let strategy = csc(values.clone(), rows.clone(), cols.clone(), max_nnz); + + let all_combinations = generate_all_possible_matrices(values, rows, cols); + + let visited_combinations = sample_matrix_output_space(strategy, + &mut runner, + num_generated_matrices); + + assert_eq!(visited_combinations.len(), all_combinations.len()); + assert_eq!(visited_combinations, all_combinations, "Did not sample all possible values."); + } + + #[cfg(feature = "slow-tests")] + #[test] + fn sparsity_pattern_samples_all_admissible_outputs() { + let mut runner = TestRunner::deterministic(); + + let num_generated_patterns = 50000; + + let major_dims = 0..=2; + let minor_dims = 0..=3; + let max_nnz = major_dims.end() * minor_dims.end(); + let strategy = sparsity_pattern(major_dims.clone(), minor_dims.clone(), max_nnz); + + let visited_patterns: HashSet<_> = sample_strategy(strategy, &mut runner) + .take(num_generated_patterns) + .map(|pattern| { + // We represent patterns as dense matrices with 1 if an entry is occupied, + // 0 otherwise + let values = vec![1; pattern.nnz()]; + CsrMatrix::try_from_pattern_and_values(pattern, values).unwrap() + }) + .map(|csr| DMatrix::from(&csr)) + .collect(); + + let all_possible_patterns = generate_all_possible_matrices(0..=1, major_dims, minor_dims); + + assert_eq!(visited_patterns.len(), all_possible_patterns.len()); + assert_eq!(visited_patterns, all_possible_patterns); + } + + fn sample_matrix_output_space(strategy: S, + runner: &mut TestRunner, + num_samples: usize) + -> HashSet> + where + S: Strategy, + DMatrix: for<'b> From<&'b S::Value> + { + sample_strategy(strategy, runner) + .take(num_samples) + .map(|matrix| DMatrix::from(&matrix)) + .collect() + } + + fn sample_strategy<'a, S: 'a + Strategy>(strategy: S, runner: &'a mut TestRunner) + -> impl 'a + Iterator { + repeat(()).map(move |_| { + let tree = strategy + .new_tree(runner) + .expect("Tree generation should not fail"); + let value = tree.current(); + value + }) + } +} \ No newline at end of file