New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add guard against exponential time pathology #1082

Merged
merged 4 commits into from Jan 24, 2018

Conversation

Projects
None yet
2 participants
@DRMacIver
Copy link
Member

DRMacIver commented Jan 22, 2018

Bad news: Hypothesis's shrinking is sometimes O(16^n).
Good news: Here's a fix for that!
Bad news: This problem is intrinsic to the problem of test-case reduction and there's no escape. All is lost. We are doomed to live in exponential land forever.

(This isn't really a problem in practice because max_shrinks saves us, but I thought I'd spend some time figuring out how to deal with the common case and it turns out that the common case is easy to deal with).

Anyway, we can detect and fix some of the common cases as follows:

  • we keep track of which bytes have changed since we last managed to reduce the size of the byte stream and as the last stage of the shrink process we try to lexically minimize them all together.
  • we beef up the way the lexical minimizer makes changes to multiple bytes at a time so it doesn't just get stuck in the same pickle.

Important note: The lexical minimizer now has quadatic complexity in the size of its input. This mostly doesn't matter because the two cases where it gets used:

  • n=16 is unusually big
  • we're in this case where we're trying to avoid an exponential slow down, so in comparison quadratic is super fast.

I do want to emphasise though that we are intrinsically doomed here. There is no non-exponential shrink algorithm that also produces good results. Something like the adversarially slow shrinker example we have in our test suite will for all intents and purposes always allow us force exponential slow downs.

So given that this PR is of somewhat questionable validity. My main arguments in favour of it are:

  1. This category of examples isn't a totally unreasonable one to optimise for and the changes that required probably help in other circumstances too (e.g. if you have size constraints among some objects)
  2. The strategy of "Oops we're failing to shrink the size, lets make this the lexicographic minimizer's problem!" is a fully general approach that allows us to tweak the lexicographic minimizer independently of the main shrinker, which I think is probably the correct thing to do for this sort of issue.
@DRMacIver

This comment has been minimized.

Copy link
Member

DRMacIver commented Jan 23, 2018

Hold off on reviewing this. I've realised a way I can make this a strict improvement on the status quo, but it needs a fairly chunky rework and I'll not get to it until later.

@DRMacIver DRMacIver force-pushed the DRMacIver/zig-zag branch from 7dd7af6 to 6a13a8d Jan 23, 2018

@DRMacIver

This comment has been minimized.

Copy link
Member

DRMacIver commented Jan 23, 2018

Hold off on reviewing this. I've realised a way I can make this a strict improvement on the status quo, but it needs a fairly chunky rework and I'll not get to it until later.

I ended up going with a completely different strict improvement on the status quo, but I'm now happy with this and think it's ready to review.

@Zac-HD

Zac-HD approved these changes Jan 24, 2018

Copy link
Member

Zac-HD left a comment

Having read your draft paper, the approach seems reasonable and the code looks good.

Your excellent write-ups of the changes in each PR continue to make reviewing much easier, so thanks for that 😄

@DRMacIver DRMacIver merged commit 6c72bd7 into master Jan 24, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@DRMacIver DRMacIver deleted the DRMacIver/zig-zag branch Jan 24, 2018

@DRMacIver

This comment has been minimized.

Copy link
Member

DRMacIver commented Jan 24, 2018

Having read your draft paper,

though sadly this example got cut for space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment