itertools icon indicating copy to clipboard operation
itertools copied to clipboard

Improve and generalize specialization testing utilities.

Open jswrenn opened this issue 2 years ago • 5 comments

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

  • @Philippe-Cholet noted that our specialization tests do not currently test partially-consumed iterators.
  • We should extend our specialization testing pattern to benchmarking, so both each specialized (and unspecialized) method are tested in a benchmark group.

jswrenn avatar Sep 28 '23 20:09 jswrenn

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

Philippe-Cholet avatar Sep 29 '23 08:09 Philippe-Cholet

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.

Philippe-Cholet avatar Sep 29 '23 10:09 Philippe-Cholet

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

jswrenn avatar Sep 29 '23 11:09 jswrenn

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

Philippe-Cholet avatar Sep 29 '23 12:09 Philippe-Cholet

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

Philippe-Cholet avatar Nov 13 '23 08:11 Philippe-Cholet