Opened 9 years ago

Closed 9 years ago

#11753 closed defect (fixed)

Fix step=0 in (x)srange

Reported by: ppurka Owned by: jason, was
Priority: minor Milestone: sage-4.7.2
Component: graphics Keywords: srange xsrange division_by_zero
Cc: jason, dsm Merged in: sage-4.7.2.alpha3
Authors: Punarbasu Purkayastha Reviewers: Dmitrii Pasechnik
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by leif)

The following command

plot(x,3,3)

generates the following error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "_sage_input_4.py", line 10, in <module>
    exec compile(u'open("___code___.py","w").write("# -*- coding: utf-8 -*-\\n" + _support_.preparse_worksheet_cell(base64.b64decode("cGxvdCh4LDMsMyk="),globals())+"\\n"); execfile(os.path.abspath("___code___.py"))
  File "", line 1, in <module>
    
  File "/tmp/tmpezK4rJ/___code___.py", line 3, in <module>
    exec compile(u'plot(x,_sage_const_3 ,_sage_const_3 )
  File "", line 1, in <module>
    
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/misc/decorators.py", line 573, in wrapper
    return func(*args, **kwds)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/misc/decorators.py", line 432, in wrapper
    return func(*args, **options)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/plot/plot.py", line 3032, in plot
    G = funcs.plot(*args, **original_opts)
  File "expression.pyx", line 8023, in sage.symbolic.expression.Expression.plot (sage/symbolic/expression.cpp:31185)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/misc/decorators.py", line 573, in wrapper
    return func(*args, **kwds)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/misc/decorators.py", line 432, in wrapper
    return func(*args, **options)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/plot/plot.py", line 3051, in plot
    G = _plot(funcs, (xmin, xmax), *args, **kwds)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/plot/plot.py", line 3154, in _plot
    data = generate_plot_points(f, xrange, plot_points, adaptive_tolerance, adaptive_recursion, randomize)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/plot/plot.py", line 4154, in generate_plot_points
    data = srange(*ranges[0], include_endpoint=True)
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/misc/misc.py", line 1149, in srange
    L = list(xsrange(start,end,step,universe,check,include_endpoint,endpoint_tolerance))
  File "/home/punarbasu/Installations/sage-4.7/local/lib/python2.6/site-packages/sage/misc/misc.py", line 1236, in xsrange
    count = (end-start)/step
ZeroDivisionError: float division

The error seems to be in [x]srange() in sage/misc/misc.py. Attached is a patch which raises a ValueError when the start point and end point of a plot are the same. The patches are to sage/{misc,plot}/misc.py and sage/plot/plot.py.

I forgot to mention earlier: The errors that are generated with this patch for [x]srange() are essentially the same errors that Python gives when [x]range() is called with step 0.


Apply only 11753-fix_div_by_zero.patch to the Sage library.

Attachments (4)

fix_div_by_zero.diff (1.3 KB) - added by ppurka 9 years ago.
New patch which doesn't silently fail and actually raises an error. Fixes are in sage/misc/misc.py and sage/plot/{plot.py,misc.py}.
sage-doctest.2.txt (10.2 KB) - added by ppurka 9 years ago.
New doctest after adding doctests
sage-doctest.txt (10.2 KB) - added by ppurka 9 years ago.
New doctest after adding doctests
11753-fix_div_by_zero.patch (4.2 KB) - added by ppurka 9 years ago.
Properly check for 0. Try 2.

Download all attachments as: .zip

Change History (23)

comment:1 follow-up: Changed 9 years ago by kcrisman

  • Cc jason dsm added

Thanks for catching this. I'll try to take a look at this sometime, though not immediately. cc:ing a couple of other people who might be interested.

A few comments:

  • Is this new behavior always desirable?
  • Might we want to catch this in plot, rather than 'fixing' misc?
  • Are there any other places this might cause problems? (E.g., do all doctests pass with this?)

I don't know the answers to these questions; this might really be the right fix, but I just want to be careful.

comment:2 in reply to: ↑ 1 Changed 9 years ago by ppurka

Replying to kcrisman:

A few comments:

  • Is this new behavior always desirable?

Having a step size of 1 should produce an empty list if the start and end points are the same, or a single element if include_endpoint=True. This patch doesn't change either of those conditions. In any other case, the step will always be strictly non-zero and the if condition in this patch will not be executed.

  • Might we want to catch this in plot, rather than 'fixing' misc?

It could be fixed in plot. I had a look at the plot function, and it seems to be a generic function which passes on the arguments to other individual plot functions (func.plot(), iirc). So, all those plot functions will have to be fixed.

  • Are there any other places this might cause problems? (E.g., do all doctests pass with this?)

I did do a doctest before submitting this bug report via

./sage -t devel/sage/sage/misc

and it passed all the tests.

The output (a rerun) is provided below:

..._10.04.1_lts-x86_64-Linux> ./sage -t -long devel/sage/sage/misc
sage -t -long "devel/sage/sage/misc/pager.py"               
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/typecheck.py"           
	 [0.1 s]
sage -t -long "devel/sage/sage/misc/sh.py"                  
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/sage_unittest.py"       
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/lazy_format.py"         
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/sage_timeit.py"         
	 [2.5 s]
sage -t -long "devel/sage/sage/misc/edit_module.py"         
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/remote_file.py"         
	 [1.8 s]
sage -t -long "devel/sage/sage/misc/misc_c.pyx"             
	 [2.4 s]
sage -t -long "devel/sage/sage/misc/lazy_import_cache.py"   
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/darwin_utilities.pyx"   
	 [1.8 s]
sage -t -long "devel/sage/sage/misc/benchmark.py"           
	 [5.0 s]
sage -t -long "devel/sage/sage/misc/sage_ostools.py"        
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/bitset.pxi"             
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/mrange.py"              
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/test_class_pickling.py" 
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/bitset.pyx"             
	 [2.2 s]
sage -t -long "devel/sage/sage/misc/getusage.py"            
	 [2.4 s]
sage -t -long "devel/sage/sage/misc/functional.py"          
	 [25.2 s]
sage -t -long "devel/sage/sage/misc/exceptions.py"          
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/hg.py"                  
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/db.py"                  
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/persist.py"             
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/randstate.pyx"          
	 [34.0 s]
sage -t -long "devel/sage/sage/misc/nested_class_test.py"   
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/function_mangling.pyx"  
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/session.pyx"            
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/sage_itertools.py"      
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/func_persist.py"        
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/flatten.py"             
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/copying.py"             
	 [0.1 s]
sage -t -long "devel/sage/sage/misc/sage_eval.py"           
	 [2.8 s]
sage -t -long "devel/sage/sage/misc/__init__.py"            
	 [0.1 s]
sage -t -long "devel/sage/sage/misc/defaults.py"            
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/random_testing.py"      
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/sagex_ds.pyx"           
	 [2.1 s]
sage -t -long "devel/sage/sage/misc/misc.py"                
	 [9.9 s]
sage -t -long "devel/sage/sage/misc/mathml.py"              
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/abstract_method.py"     
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/parser.pyx"             
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/prandom.py"             
	 [3.4 s]
sage -t -long "devel/sage/sage/misc/file_to_worksheet.py"   
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/all.py"                 
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/explain_pickle.py"      
	 [2.2 s]
sage -t -long "devel/sage/sage/misc/preparser_ipython.py"   
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/sage_timeit_class.pyx"  
	 [7.5 s]
sage -t -long "devel/sage/sage/misc/lazy_attribute.py"      
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/preparser.py"           
	 [8.0 s]
sage -t -long "devel/sage/sage/misc/derivative.pyx"         
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/unknown.py"             
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/cython.py"              
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/multireplace.py"        
	 [1.8 s]
sage -t -long "devel/sage/sage/misc/cython_c.pyx"           
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/profiler.py"            
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/constant_function.py"   
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/inline_fortran.py"      
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/attach.py"              
	 [1.8 s]
sage -t -long "devel/sage/sage/misc/lazy_import.pyx"        
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/method_decorator.py"    
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/trace.py"               
	 [3.7 s]
sage -t -long "devel/sage/sage/misc/map_threaded.py"        
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/bitset_pxd.pxi"         
	 [0.1 s]
sage -t -long "devel/sage/sage/misc/interpreter.py"         
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/decorators.py"          
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/refcount.pyx"           
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/nested_class.py"        
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/latex.py"               
	 [3.6 s]
sage -t -long "devel/sage/sage/misc/citation.pyx"           
	 [4.5 s]
sage -t -long "devel/sage/sage/misc/html.py"                
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/python.py"              
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/displayhook.py"         
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/reset.pyx"              
	 [4.0 s]
sage -t -long "devel/sage/sage/misc/sage_input.py"          
	 [2.6 s]
sage -t -long "devel/sage/sage/misc/object_multiplexer.py"  
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/allocator.pyx"          
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/bug.py"                 
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/package.py"             
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/test_cpickle_sage.py"   
	 [0.1 s]
sage -t -long "devel/sage/sage/misc/proof.py"               
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/fpickle.pyx"            
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/sagedoc.py"             
	 [47.1 s]
sage -t -long "devel/sage/sage/misc/cachefunc.py"           
	 [4.6 s]
sage -t -long "devel/sage/sage/misc/classgraph.py"          
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/sageinspect.py"         
	 [2.8 s]
sage -t -long "devel/sage/sage/misc/log.py"                 
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/search.pyx"             
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/dist.py"                
	 [2.1 s]
sage -t -long "devel/sage/sage/misc/classcall_metaclass.py" 
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/cache.py"               
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/banner.py"              
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/latex_macros.py"        
	 [1.9 s]
sage -t -long "devel/sage/sage/misc/viewer.py"              
	 [2.0 s]
sage -t -long "devel/sage/sage/misc/pickle_old.pyx"         
	 [1.9 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 307.3 seconds

Changed 9 years ago by ppurka

New patch which doesn't silently fail and actually raises an error. Fixes are in sage/misc/misc.py and sage/plot/{plot.py,misc.py}.

comment:3 Changed 9 years ago by ppurka

Attached patch which patches srange() and xsrange() in devel/sage/sage/misc/misc.py and setup_for_eval_on_grid() in devel/sage/sage/plot/misc.py

In all the functions, the patch will now raise an error instead of silently failing.

The plot() command (and also plot3d()) both now give the a more meaningful error if the endpoint and startpoints in the range are the same.

comment:4 follow-up: Changed 9 years ago by dimpase

Please add plot(x,3,3) to the appropriate doctest, to show that the patch really does the trick.

Changed 9 years ago by ppurka

New doctest after adding doctests

Changed 9 years ago by ppurka

New doctest after adding doctests

comment:5 in reply to: ↑ 4 ; follow-up: Changed 9 years ago by ppurka

Replying to dimpase:

Please add plot(x,3,3) to the appropriate doctest, to show that the patch really does the trick.

Updated the patch with doctests.

comment:6 Changed 9 years ago by ppurka

  • Description modified (diff)

comment:7 in reply to: ↑ 5 ; follow-up: Changed 9 years ago by dimpase

  • Status changed from new to needs_info

Replying to ppurka:

Replying to dimpase:

Please add plot(x,3,3) to the appropriate doctest, to show that the patch really does the trick.

Updated the patch with doctests.

in misc.py, can't you do map(len,ranges) rather than fortranic [len(r) for r in ranges] ?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 9 years ago by ppurka

Replying to dimpase:

in misc.py, can't you do map(len,ranges) rather than fortranic [len(r) for r in ranges] ?

I haven't touched that code except to put opening and closing braces [ ]. Without the braces it was giving me EOF errors and was failing the doctests.

comment:9 Changed 9 years ago by ppurka

  • Description modified (diff)
  • Status changed from needs_info to needs_review

comment:10 in reply to: ↑ 8 ; follow-up: Changed 9 years ago by dimpase

Replying to ppurka:

Replying to dimpase:

in misc.py, can't you do map(len,ranges) rather than fortranic [len(r) for r in ranges] ?

I haven't touched that code except to put opening and closing braces [ ]. Without the braces it was giving me EOF errors and was failing the doctests.

The condition step == 0 you check looks worrisome. From a glance over the code it's not clear whether step cannot be a float, and if it is a float, then such a test does not make much sense.

comment:11 in reply to: ↑ 10 ; follow-up: Changed 9 years ago by ppurka

Replying to dimpase:

The condition step == 0 you check looks worrisome. From a glance over the code it's not clear whether step cannot be a float, and if it is a float, then such a test does not make much sense.

Should be fixed now.

Changed 9 years ago by ppurka

Properly check for 0. Try 2.

comment:12 in reply to: ↑ 11 ; follow-up: Changed 9 years ago by dimpase

  • Status changed from needs_review to needs_work

Replying to ppurka:

Replying to dimpase:

The condition step == 0 you check looks worrisome. From a glance over the code it's not clear whether step cannot be a float, and if it is a float, then such a test does not make much sense.

Should be fixed now.

Ever written "real" numerical code?

min(range_steps) == float(0) won't fly. You need something like min(range_steps)<C*sys.float_info.epsilon, for C something like 2 or 10...

comment:13 in reply to: ↑ 12 ; follow-up: Changed 9 years ago by ppurka

Replying to dimpase:

Replying to ppurka:

Replying to dimpase:

The condition step == 0 you check looks worrisome. From a glance over the code it's not clear whether step cannot be a float, and if it is a float, then such a test does not make much sense.

Should be fixed now.

Ever written "real" numerical code?

min(range_steps) == float(0) won't fly. You need something like min(range_steps)<C*sys.float_info.epsilon, for C something like 2 or 10...

Is this needed? Something like plot(sqrt(x), 0, sys.float_info.epsilon**2) works just fine.

comment:14 in reply to: ↑ 13 Changed 9 years ago by dimpase

Replying to ppurka:

Replying to dimpase:

Replying to ppurka:

Replying to dimpase:

The condition step == 0 you check looks worrisome. From a glance over the code it's not clear whether step cannot be a float, and if it is a float, then such a test does not make much sense.

Should be fixed now.

Ever written "real" numerical code?

min(range_steps) == float(0) won't fly. You need something like min(range_steps)<C*sys.float_info.epsilon, for C something like 2 or 10...

Is this needed? Something like plot(sqrt(x), 0, sys.float_info.epsilon**2) works just fine.

oops, sorry. It looks like plot works with arbitrary precision floats from sage.rings.real_mpfr, not with hardware plots! Then indeed, your previous version was just fine. (no need to do any float(0) either)

comment:15 Changed 9 years ago by ppurka

  • Status changed from needs_work to needs_review

comment:16 Changed 9 years ago by dimpase

  • Status changed from needs_review to positive_review

comment:17 Changed 9 years ago by leif

  • Authors set to P. Purkayastha
  • Description modified (diff)
  • Reviewers set to Dmitrii Pasechnik

comment:18 Changed 9 years ago by ppurka

  • Authors changed from P. Purkayastha to Punarbasu Purkayastha

comment:19 Changed 9 years ago by leif

  • Merged in set to sage-4.7.2.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.