From 6e10efe862f9a1daedce6a417221477d39d49b13 Mon Sep 17 00:00:00 2001 From: Andreas Longva Date: Wed, 1 Dec 2021 12:44:07 +0100 Subject: [PATCH] Remove redundant proptest patch --- nalgebra-sparse/src/proptest.rs | 19 +-- .../src/proptest/proptest_patched.rs | 146 ------------------ 2 files changed, 4 insertions(+), 161 deletions(-) delete mode 100644 nalgebra-sparse/src/proptest/proptest_patched.rs diff --git a/nalgebra-sparse/src/proptest.rs b/nalgebra-sparse/src/proptest.rs index 472c466f..93db7796 100644 --- a/nalgebra-sparse/src/proptest.rs +++ b/nalgebra-sparse/src/proptest.rs @@ -5,11 +5,6 @@ //! The strategies provided here are generally expected to be able to generate the entire range //! of possible outputs given the constraints on dimensions and values. However, there are no //! particular guarantees on the distribution of possible values. - -// 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 crate::csc::CscMatrix; use crate::csr::CsrMatrix; @@ -31,16 +26,10 @@ fn dense_row_major_coord_strategy( let mut booleans = vec![true; nnz]; booleans.append(&mut vec![false; (nrows * ncols) - nnz]); // Make sure that exactly `nnz` of the booleans are true - - // 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| { + Just(booleans) + // Need to shuffle to make sure they are randomly distributed + .prop_shuffle() + .prop_map(move |booleans| { booleans .into_iter() .enumerate() diff --git a/nalgebra-sparse/src/proptest/proptest_patched.rs b/nalgebra-sparse/src/proptest/proptest_patched.rs deleted file mode 100644 index 37c71262..00000000 --- a/nalgebra-sparse/src/proptest/proptest_patched.rs +++ /dev/null @@ -1,146 +0,0 @@ -//! 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::num; -use proptest::prelude::Rng; -use proptest::strategy::{NewTree, Shuffleable, Strategy, ValueTree}; -use proptest::test_runner::{TestRng, TestRunner}; -use std::cell::Cell; - -#[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() - } - } -}