rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.77k stars 309 forks source link

Improve and generalize specialization testing utilities. #766

Open jswrenn opened 1 year ago

jswrenn commented 1 year ago

This issue tracks two complimentary directions for improving our specialization tests:

Philippe-Cholet commented 1 year ago

I'm definitely gonna work on improving specialization tests (as mentioned) and benchmarks (at some point).

Philippe-Cholet commented 1 year ago

After working on combinations & co which produce a lot of elements, a "struggle" I sometimes encounter when writing quickcheck tests is to have small enough iterators on which apply our itertools adaptors (otherwise tests would be too slow). The current iterators we rely on are mostly the custom Iter<u8, Inexact(default) or Exact> and Vec<u8>. We can call .take(...) on Iter (but eventually lose the unfused element at the end) and truncate vectors. But it surely does not reduce the numbers of tests. Let's say it tests against vectors of lengths 0..=128 and we truncate each to 10 inside the test then it tests against a lot of iterators of length 10, mostly unnecessarily. I think the problem is that the truncation is done too late to reduce the number of tests.

So I think it would be valuable to have a way to generate small iterators, especially for specialization tests which I expect to be slower if we want to test against more than fresh iterators. Even without const generics Iter<T, const SIZE_LIMIT: usize>, I'm sure we could have Iter<T, LIMIT=NoLimit> or Iter<T, Max10>. And similarly wraps vectors into struct LittleVec<T, LIMIT=Max10>(Vec<T>);. Maybe LittleVec would be enough. Maybe you have a better idea.

This is merely a though I had, I did not work on it yet.

jswrenn commented 1 year ago

The simplest thing we can do, right now, is to return, rather than truncate. That won't totally discard the test, but it will avoid most of the computational overhead of running it.

To totally discard a test, see this guidance in the quickcheck readme: https://github.com/BurntSushi/quickcheck#discarding-test-results-or-properties-are-polymorphic

Philippe-Cholet commented 1 year ago

Thanks! ...I guess I simply forgot about TestResult, autocompletion would have then saved me. 😥

Philippe-Cholet commented 1 year ago

If #786 is not enough to extend our specialization benchmarking, we'll make another PR.