Opened 15 years ago

Closed 14 years ago

#3476 closed enhancement (fixed)

[with patch, positive review] save timeit information with sage -t -timeit

Reported by: ncalexan Owned by: failure
Priority: major Milestone: sage-3.2
Component: doctest coverage Keywords: testing doctest timing timeit profile regression, editor_mhansen
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

There are several parts to this patch.

The first is an update to sage.misc.sage_timeit that generalizes the interface to return an object that prints itself as a timing string rather than the string itself. The advantage is that the information does not need to be parsed from the string later. This patch is a requirement of the later ones but is conceptually independent.

The second adds a file ncadoctest.py to scripts that is a slightly modified version of Python's doctest.py file. It is easier to subclass the various classes with this version.

The third uses ncadoctest.py to subclass the doctest architecture and updates sage-doctest to use these updated classes.

Attachments (5)

3476-ncalexan-sage-scripts-doctest-1.patch (105.8 KB) - added by ncalexan 15 years ago.
3476-ncalexan-sage-timeit-1.patch (1.9 KB) - added by ncalexan 15 years ago.
diff-python-doctest-to-ncadoctest (2.1 KB) - added by ncalexan 15 years ago.
trac_3476.patch (1.7 KB) - added by mhansen 14 years ago.
trac_3476-scripts.patch (105.5 KB) - added by mhansen 14 years ago.

Download all attachments as: .zip

Change History (20)

Changed 15 years ago by ncalexan

Changed 15 years ago by ncalexan

comment:1 Changed 15 years ago by ncalexan

The attachments came in the wrong order -- the one with sage-scripts applies to sage/local/bin.

Changed 15 years ago by ncalexan

comment:2 Changed 15 years ago by ncalexan

diff-python-doctest-to-ncadoctest records the changes from upstream Python doctest.py to ncadoctest.py for future reference.

comment:3 Changed 15 years ago by mabshoff

Keywords: editor_mabshoff added

comment:4 Changed 14 years ago by gfurnish

Keywords: editor_gfurnish added

comment:5 Changed 14 years ago by rlm

This should be applied after #3982.

comment:6 Changed 14 years ago by rlm

Keywords: editor_mhansen added; editor_mabshoff editor_gfurnish removed

comment:7 Changed 14 years ago by mhansen

Summary: [with patch, needs review] save timeit information with sage -t -timeit[with patch, positive review] save timeit information with sage -t -timeit

Since my one main concern at #3982 is taken care of, I think this can go in.

comment:8 Changed 14 years ago by mabshoff

There is a reject apllying Nick's first patch:

--- sage-doctest
+++ sage-doctest
@@ -218,8 +241,8 @@ def extract_doc(file_name, module):
             doc = doc_preparse(F[i:j+3])
         except SyntaxError:
             doc = F[i:j+3]
-        if len(doc):
-            doc = '""">>> set_random_seed(0L)\n\n' + doc[3:]
+#         if len(doc):
+#             doc = '""">>> print "YYY"; print random() # ; set_random_seed(0L)\n\n' + doc[3:]
         s += "\tr"+ doc + "\n\n"
         F = F[j+3:]

I am attempting to merge this manually.

Cheers,

Michael

comment:9 Changed 14 years ago by mabshoff

Ok, the reject seems to happen due to merging the warning patch into sage-doctest.

Cheers,

Michael

comment:10 Changed 14 years ago by mabshoff

Summary: [with patch, positive review] save timeit information with sage -t -timeit[with patch, needs works] save timeit information with sage -t -timeit

If I merge the patch without the troublesome hunk I see 4 doctests related to the random framework and timeit itself fail:

	sage -t -long devel/sage/sage/misc/sage_timeit_class.pyx # 7 doctests failed
	sage -t -long devel/sage/sage/misc/sage_timeit.py # 2 doctests failed
	sage -t -long devel/sage/sage/misc/prandom.py # 2 doctests failed
	sage -t -long devel/sage/sage/misc/randstate.pyx # 6 doctests failed

Oh well, life sucks :)

Cheers,

Michael

comment:11 Changed 14 years ago by mabshoff

Summary: [with patch, needs works] save timeit information with sage -t -timeit[with patch, needs work] save timeit information with sage -t -timeit

Changed 14 years ago by mhansen

Attachment: trac_3476.patch added

Changed 14 years ago by mhansen

Attachment: trac_3476-scripts.patch added

comment:12 Changed 14 years ago by mhansen

Summary: [with patch, needs work] save timeit information with sage -t -timeit[with patch, positive review] save timeit information with sage -t -timeit

I've added two updated patches which fix the issue. The problem was that the 'timeit' in test.globs was set to "False" from the options in sage-doctest instead of being the actual timeit function from Sage. Thus, you'd only hit the problem with doctests that used timeit.

comment:13 Changed 14 years ago by mabshoff

Milestone: sage-3.2.1sage-3.2

For "sage -sdist" to work we need to copy ncadoctest.py, sagedoctest.py in sage-make_devel_packages after

  cp -p $SAGE_ROOT/local/bin/SbuildHack.pm $SCRIPTS/

I will take care of this once the patch passes doctests.

Cheers,

Michael

comment:14 Changed 14 years ago by mabshoff

Hi Mike,

there is one tiny easy to fix doctest issue left:

sage -t -long devel/sage/sage/misc/sage_timeit.py           
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.2.rc0/devel/sage/sage/misc/sage_timeit.py", line 48:
    sage: sage_timeit("a = 2\nb=131\nfactor(a^b-1)", globals(), number=10)
Expected:
    '10 loops, best of 3: ... per loop'
Got:
    10 loops, best of 3: 18.4 ms per loop
**********************************************************************

I will fix this via a followup patch.

Cheers,

Michael

comment:15 Changed 14 years ago by mabshoff

Resolution: fixed
Status: newclosed

Merged in Sage 3.2.rc0

Note: See TracTickets for help on using tickets.