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

[WIP] Added timeout_after context manager #8297

Closed
wants to merge 1 commit into from

Conversation

jcrist
Copy link
Member

@jcrist jcrist commented Oct 24, 2014

Note: This is just a proof of concept. See #8295 for another option of implementing. I like this one better, but both attempt to solve the same issue.

Added contextmanager to make timeouts on sympy calls available.

Example:

>>> # Timeout after 3 seconds
>>> with timeout_after(3):
        expr = simplify(expr)

Also added a timeout kwarg to Expr.expand and expand as an example
of how the manager can be used to apply to functions.

caveats

  • I picked _ask as a good function to check, because it's called a lot. However, this may (probably will) add considerable overhead to sympy. The best option would be to find a set of functions that are called often enough, but not too often.
  • This uses a global variable, and is thus not threadsafe. However, other things (evaluate, etc...) also use globals for the same effect, so sympy already isn't threadsafe.
  • This doesn't allow for the resolution that [WIP] Added timeout argument to fu #8295 does, but is incredibly easier to use.

Added contextmanager to make timeouts on sympy calls available.

Example:

```
>>> # Timeout after 3 seconds
>>> with timeout_after(3):
        expr = simplify(expr)
```

Also added a `timeout` kwarg to `Expr.expand` and `expand` as an example
of how the manager can be used to apply to functions.
@jcrist
Copy link
Member Author

jcrist commented Oct 25, 2014

Some timings:

Running bin/test core

Without this PR: 280.29 seconds

With this PR: 298.19 seconds

So about 6.5% slower for core tests.

Running bin/test series

Without this PR: 606.08 seconds

With this PR: 606.47 seconds

So a negligible difference.


I'm still not sure this is even a good idea, and it relies on SymPy having a function that is called periodically to allow for timing checks. Just food for discussion.

@debugger22
Copy link
Member

We can think of a model where this feature activates only when it is needed.

Are you planning to finish this?

I'm still not sure this is even a good idea

@asmeurer Should we go ahead with this feature?

@jcrist
Copy link
Member Author

jcrist commented Feb 21, 2015

My intended purpose here was to allow for a way to terminate calls that may run for an unpredictable amount of time (simplify, integrate, etc...). Unfortunately, without a callback, there is no cross-platform way to implement a timeout in a robust way. I'm still not sure if this is a good idea (added overhead), and I think our time may be better spent improving these long running functions. Still, it would allow library code to use simplify without fear of it locking up.

@moorepants
Copy link
Member

Another option would be to lazily count the operations in an expression. If the count is above some value, don't try to simplify.

@jcrist
Copy link
Member Author

jcrist commented May 27, 2015

Closing. I no longer see this as valuable, and it's definitely not the best way forward.

@jcrist jcrist closed this May 27, 2015
@jcrist jcrist deleted the timeout_2 branch May 27, 2015 18:42
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.

None yet

3 participants