Skip to content
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

Setting for shrinker deadline #3400

Conversation

rhaps0dy
Copy link

@rhaps0dy rhaps0dy commented Jul 8, 2022

Currently the shrinker gives up after 5 minutes (#3243 ), and this number is hardcoded.

#3243 states that you don't want to expose that as a user setting, contra this PR. Is it because it encourages complacency in the runtime of strategies?

Testing: I found tests/conjecture/test_engine.py::test_exit_because_shrink_phase_timeout, but couldn't figure out how to make it check that the deadline is responsive to the setting. Maybe making f stateful and it needs to be called N times before it marks the data as interesting?

Enhancements: I've thrown together a minimal change for my own use, but I can make this setting a timedelta similar to deadline if the PR is wanted.

@Zac-HD
Copy link
Member

Zac-HD commented Jul 8, 2022

We don't want to expose this because the shrinker is basically an internal implementation detail, and we only want settings for things that can only be the responsibility of the user. Presenting many options is actually a worse user experience than fewer, especially if they require understanding our internals to decide an appropriate value.

Separately, why do you want this feature? The five-minute timeout should almost never arise in practice.

@Zac-HD Zac-HD closed this Jul 8, 2022
@nchammas
Copy link
Contributor

Separately, why do you want this feature? The five-minute timeout should almost never arise in practice.

I hit this timeout regularly when testing Apache Spark, which is unfortunately dog slow. It's designed for terabyte to petabyte scale operations, so low latency on individual DataFrame operations is not a priority.

Here are the stats from a test run I just did:

  - during shrink phase (303.50 seconds):
    - Typical runtimes: ~ 2495-7002 ms, of which < 1ms in data generation
    - 5 passing examples, 10 failing examples, 50 invalid examples
    - Tried 65 shrinks of which 9 were successful

  - Stopped because shrinking was very slow

I don't know if this makes the case for a user-configurable shrink timeout, but this is my experience.

I would find it convenient if I could disable the shrink timeout and tell Hypothesis to keep churning until it cannot make shrink progress anymore.

@Zac-HD
Copy link
Member

Zac-HD commented Feb 18, 2024

Ouch! I'm still disinclined to make it a public setting, but we could pull out the 300 to a module-level constant that you can monkeypatch...

@nchammas
Copy link
Contributor

I wouldn't make any changes on my account if I'm the only one with this use case. But perhaps if someone publicly documents an additional use case, then yes, giving us sloth herders some hack around the time limit would be helpful.

@nchammas
Copy link
Contributor

but we could pull out the 300 to a module-level constant that you can monkeypatch...

As a form of unofficial documentation for others reading along, the new constant added in Hypothesis 6.98.8 is hypothesis.internal.conjecture.engine.MAX_SHRINKING_SECONDS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants