Ticket #3476 (closed enhancement: fixed)

Opened 6 months ago

Last modified 3 weeks ago

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

Reported by: ncalexan Assigned to: failure
Priority: major Milestone: sage-3.2
Component: doctest Keywords: testing doctest timing timeit profile regression, editor_mhansen
Cc:

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

3476-ncalexan-sage-scripts-doctest-1.patch (105.8 kB) - added by ncalexan on 06/19/2008 02:54:31 PM.
3476-ncalexan-sage-timeit-1.patch (1.9 kB) - added by ncalexan on 06/19/2008 02:55:11 PM.
diff-python-doctest-to-ncadoctest (2.1 kB) - added by ncalexan on 06/19/2008 03:17:40 PM.
trac_3476.patch (1.7 kB) - added by mhansen on 11/07/2008 09:05:18 PM.
trac_3476-scripts.patch (105.5 kB) - added by mhansen on 11/07/2008 09:05:41 PM.

Change History

06/19/2008 02:54:31 PM changed by ncalexan

  • attachment 3476-ncalexan-sage-scripts-doctest-1.patch added.

06/19/2008 02:55:11 PM changed by ncalexan

  • attachment 3476-ncalexan-sage-timeit-1.patch added.

06/19/2008 02:56:06 PM changed by ncalexan

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

06/19/2008 03:17:40 PM changed by ncalexan

  • attachment diff-python-doctest-to-ncadoctest added.

06/19/2008 03:18:26 PM changed by ncalexan

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

07/06/2008 03:56:48 AM changed by mabshoff

  • keywords changed from testing doctest timing timeit profile regression to testing doctest timing timeit profile regression, editor_mabshoff.

08/13/2008 08:09:53 PM changed by gfurnish

  • keywords changed from testing doctest timing timeit profile regression, editor_mabshoff to testing doctest timing timeit profile regression, editor_mabshoff, editor_gfurnish.

08/28/2008 04:34:40 PM changed by rlm

This should be applied after #3982.

08/28/2008 04:44:54 PM changed by rlm

  • keywords changed from testing doctest timing timeit profile regression, editor_mabshoff, editor_gfurnish to testing doctest timing timeit profile regression, editor_mhansen.

09/03/2008 08:49:37 PM changed by mhansen

  • summary changed from [with patch, needs review] save timeit information with sage -t -timeit to [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.

09/04/2008 04:37:40 PM changed 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

09/04/2008 04:43:41 PM changed by mabshoff

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

Cheers,

Michael

09/04/2008 05:06:44 PM changed by mabshoff

  • summary changed from [with patch, positive review] save timeit information with sage -t -timeit to [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

09/28/2008 05:14:49 PM changed by mabshoff

  • summary changed from [with patch, needs works] save timeit information with sage -t -timeit to [with patch, needs work] save timeit information with sage -t -timeit.

11/07/2008 09:05:18 PM changed by mhansen

  • attachment trac_3476.patch added.

11/07/2008 09:05:41 PM changed by mhansen

  • attachment trac_3476-scripts.patch added.

11/07/2008 09:07:39 PM changed by mhansen

  • summary changed from [with patch, needs work] save timeit information with sage -t -timeit to [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.

11/08/2008 12:00:52 AM changed by mabshoff

  • milestone changed from sage-3.2.1 to sage-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

11/08/2008 09:22:53 AM changed 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

11/08/2008 12:11:23 PM changed by mabshoff

  • status changed from new to closed.
  • resolution set to fixed.

Merged in Sage 3.2.rc0