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

Create a stopgap warning #12691

Closed
roed314 opened this issue Mar 18, 2012 · 15 comments
Closed

Create a stopgap warning #12691

roed314 opened this issue Mar 18, 2012 · 15 comments

Comments

@roed314
Copy link
Contributor

roed314 commented Mar 18, 2012

Sometimes issues can produce mathematically incorrect results, and we should warn users of the problem. If the issue is fixed promptly then nothing more needs to be done, but sometimes tickets remain unresolved for months or years (c.f. #11358). This ticket adds a function that makes it easy to create a second ticket warning users of the problem while it's being resolved.

On trac, this functionality will be supported by the Stopgaps field. Create a new ticket that uses the stopgap function and include a link to it in the Stopgaps field in the original ticket.


Apply attachment: trac_12691-faster.patch

Depends on #12702

Component: misc

Keywords: rd2

Author: David Roe, William Stein

Reviewer: R. Andrew Ohana

Merged: sage-5.0.beta10

Issue created by migration from https://trac.sagemath.org/ticket/12691

@roed314
Copy link
Contributor Author

roed314 commented Mar 19, 2012

Attachment: 12691.patch.gz

@jbalakrishnan
Copy link

comment:2

Looks good.

@jbalakrishnan
Copy link

Changed keywords from none to rd2

@jhpalmieri
Copy link
Member

comment:3

See #12702 for a followup.

@williamstein
Copy link
Contributor

comment:4

I think this code is too slow. We want it to be highly optimized so as to not have much of a performance penalty for introducing a stopgap. I'm going to attach a different patch that implements stopgaps, and is an order of magnitude faster than this code:

sage: timeit("sage.misc.misc.stopgap('abc', 123)", number=10^5)
100000 loops, best of 3: 2.49 µs per loop
sage: import sage.misc.stopgap
sage: timeit("sage.misc.stopgap.stopgap('abc', 123)", number=10^5)
100000 loops, best of 3: 388 ns per loop
sage: 2.49 / .388
6.41752577319588

@ohanar
Copy link
Member

ohanar commented Mar 20, 2012

Changed author from David Roe to David Roe, William Stein

@ohanar
Copy link
Member

ohanar commented Mar 20, 2012

comment:7

Attachment: trac_12691-faster.patch.gz

everything looks good to me and works how I expect it to, positive review!

@ohanar
Copy link
Member

ohanar commented Mar 20, 2012

Reviewer: R. Andrew Ohana

@ohanar

This comment has been minimized.

@kcrisman
Copy link
Member

comment:9

Just clarification; this doesn't

create a second ticket warning users of the problem

right, or even 'make it easy' to do so? It seems to function more like deprecations; it doesn't do anything, but warns users that something might be wrong. Let me know if I'm wrong about this, otherwise I hope to use it in the future.

Nice addition.

@jhpalmieri
Copy link
Member

comment:10

kcrisman: you're right, it doesn't create a ticket or make it easy to create a ticket, but it makes it easy to print a nice warning, like a deprecation. Does the documentation at #12702 make it clearer?

@kcrisman
Copy link
Member

comment:11

Yeah, that's great. Wish I could be helping with this, but looks like RD2 is going very well.

@jhpalmieri
Copy link
Member

Dependencies: #12702

@jdemeyer
Copy link

Merged: sage-5.0.beta10

@jdemeyer
Copy link

jdemeyer commented Apr 3, 2012

comment:14

Why not display the stopgap warnings in doctests? I would prefer documentation to show the stopgap warnings. It also would allow to see which parts of Sage are affected by stopgapped tickets.

It would also prevent sillyness like

sage: 1+1
********************************************************************************
Addition of integers can sometimes return wrong results.
This issue is being tracked at https://github.com/sagemath/sage/issues/123456.
********************************************************************************
2

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

No branches or pull requests

8 participants