Warn about long doctests (#16609 needs review)

[Volker Braun]

A small handful of modules takes the majority of the doctest walltime, see e.g. this lopsided distribution:

I'm proposing to 

* turn the existing sage -t --warn-long parameter into a warning (instead of an error)

* enable it by default with a limit of 60s on a modern computer, adjusted for slowness of the computer running the tests.

This is implemented in http://trac.sagemath.org/ticket/16609. Please review.

[John Cremona]

I would not be surprised if some of the offending long tests are by me (" just because the author wants to show off the capabilities of their code")  so can you post a list of the longest ones?

The other factor is that some modules are themselves very long (thousands of lines) withough -- necessarily -- having long individual tests.  Though probably they do.

John

--
You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To post to this group, send email to sage-...@googlegroups.com.
Visit this group at http://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.

[Volker Braun]

sage: stats = json.load(open('/home/vbraun/.sage/timings2.json'))

sage: sorted([(v['walltime'], k) for k,v in stats.items() if 'failed' not in v])[-20:]

[(103.71118903160095,

  u'sage.combinat.rigged_configurations.tensor_product_kr_tableaux'),

 (105.40626382827759, u'sage.combinat.root_system.plot'),

 (105.58554887771606, u'sage.functions.bessel'),

 (106.15052008628845, u'sage.plot.graphics'),

 (108.91321992874146, u'sage.schemes.elliptic_curves.isogeny_small_degree'),

 (113.79727506637573, u'sage.matroids.catalog'),

 (117.22659015655518, u'sage.symbolic.random_tests'),

 (118.7537989616394, u'sage.combinat.ncsf_qsym.ncsf'),

 (125.5258629322052, u'sage.combinat.backtrack'),

 (126.43836998939514,

  u'sage.schemes.hyperelliptic_curves.hyperelliptic_finite_field'),

 (127.17231702804565, u'sage.plot.plot'),

 (128.25516295433044, u'sage.calculus.riemann'),

 (135.7807331085205,

  u'sage.combinat.rigged_configurations.rigged_configurations'),

 (149.45724391937256, u'sage.tests.book_schilling_zabrocki_kschur_primer'),

 (158.25851893424988, u'sage.combinat.sf.macdonald'),

 (171.6492350101471, u'sage.combinat.crystals.alcove_path'),

 (203.31193494796753, u'sage.schemes.elliptic_curves.ell_rational_field'),

 (214.14894604682922, u'sage.combinat.cluster_algebra_quiver.mutation_type'),

 (271.6959960460663, u'sage.combinat.similarity_class_type'),

 (375.5583908557892,

  u'sage.combinat.root_system.non_symmetric_macdonald_polynomials')]

We don't need to change anything in these right away, hopefully the warning will nag the authors into shortening their test ;-)  Maybe with the exception of sage.plot.graphics, none of these are core functionality making it very likely that they just demonstrate what can be done instead of being necessary for coverage.

[John Cremona]

OK, so I may have some responsibility for 2 of those:

(108.91321992874146, u'sage.schemes.elliptic_

curves.isogeny_small_degree'),

(203.31193494796753, u'sage.schemes.elliptic_

curves.ell_rational_field'),

and it should be easy to reduce the load for the first one.

I doubt that nagging will work!

John

[Ralf Stephan]

On Tuesday, July 29, 2014 2:32:43 PM UTC+2, Volker Braun wrote:

 (105.58554887771606, u'sage.functions.bessel'),

It's just four complex_plots that need all long time (70% of total) here.

Regards, 

[Travis Scrimshaw]

Is this with the libgap speedup? I know a bunch of those end up importing libgap (usually through a Weyl group).

The tests in rigged_configurations are there for coverage (and have been useful for catching bugs). I can look into some of the other combinat ones.

Best,

Travis

[Volker Braun]

This is with the libgap saved workspace (and initializing libgap takes only one second on my laptop anyways)

[Volker Braun]

On Tuesday, July 29, 2014 4:20:09 AM UTC-4, John Cremona wrote:

The other factor is that some modules are themselves very long (thousands of lines) withough -- necessarily -- having long individual tests.  Though probably they do.

Only the time needed for a single test is used to decide to print a warning or not. Many short doctests will never trigger it, no matter how many tests there are in the module. For example:

sage -t --long --warn-long 54.1 src/sage/calculus/riemann.pyx

**********************************************************************

File "src/sage/calculus/riemann.pyx", line 1036, in sage.calculus.riemann.Riemann_Map.plot_colored

Warning, slow doctest:

    m.plot_colored(plot_points=1000)  # long time (29s on sage.math, 2012)

Test ran for 66.52 s

    [174 tests, 128.26 s]

sage -t --long --warn-long 54.1 src/sage/combinat/similarity_class_type.py

**********************************************************************

What do we test with 1000 point resolution that can't be tested with 10?

[P Purkayastha]

Hmm.. sage.plot.{graphics,plot} can not be avoided. We need to show a lot of examples to the users, since the number of possible customizations is huge.

[Volker Braun]

On Tuesday, July 29, 2014 8:59:56 PM UTC-4, P Purkayastha wrote:

Hmm.. sage.plot.{graphics,plot} can not be avoided. We need to show a lot of examples to the users, since the number of possible customizations is huge.

And they don't trigger the warnings since each individual tests is relatively short. Still, some of the high plot_points options should be reduced to the default...

[Ralf Stephan]

I thought setting plot_points=10 would give some ugly chunky

image but funnily it's nearly indistinguishable, and much faster.

http://trac.sagemath.org/ticket/16737

Please review.

Regards,

[Frédéric Chapoton]

I have made ticket #16738 for removing the long tests in src/sage/combinat/cluster_algebra_quiver/mutation_type.py

[Niles Johnson]

On Tuesday, July 29, 2014 2:24:10 PM UTC-4, Volker Braun wrote:

What do we test with 1000 point resolution that can't be tested with 10?

Maybe more examples should be flagged with "# not tested" or something like "# optional -- demo" or "# optional -- tutorial"?

This and another recent thread have clarified for me that the doctests serve two goals which often overlap but are actually distinct:

1. Test that code produces the expected output

2. Document what sage can do

The 1000 point resolution is not so useful for 1, but is useful for 2 -- it illustrates that sage can produce very high quality graphics.  A number of the other long doctests are probably more useful for 2 than 1.  Rather than remove them, it seems better to flag them in some way so they're not tested.

Unfortunately, I think the wording of the "# not tested" flag is misleading for novice users (a large part of the target audience) -- it might be mistaken to mean that the example is not expected to work, or hasn't ever been tested by anyone.  I don't think there's any harm in some version of the "# optional" flag, except we'd have to agree on yet another convention that we'll instantly forget, or agree to just use "# optional -- [whatever strikes my fancy, so long as it doesn't conflict with the other optional flags]".

Surely this kind of problem has come up elsewhere in the Python community -- is anyone familiar with it?

-Niles

p.s.  It occurs to me that if we do agree on a new optional flag, then these examples could be tested every so often if one wanted.

[Volker Braun]

On Wednesday, July 30, 2014 11:04:02 AM UTC-4, Niles Johnson wrote:

1. Test that code produces the expected output

2. Document what sage can do

The 1000 point resolution is not so useful for 1, but is useful for 2 -- it illustrates that sage can produce very high quality graphics

IMHO the doctest should test that the plot_points optional keyword argument is accepted and works. Does it really require an example to illustrate that more points yields more accurate graphics?

[Niles Johnson]

For this particular example, I think you're right -- I should have chosen a better example to illustrate my point.

[kcrisman]

1. Test that code produces the expected output

2. Document what sage can do

The 1000 point resolution is not so useful for 1, but is useful for 2 -- it illustrates that sage can produce very high quality graphics

IMHO the doctest should test that the plot_points optional keyword argument is accepted and works. Does it really require an example to illustrate that more points yields more accurate graphics?

For this particular example, I think you're right -- I should have chosen a better example to illustrate my point.

I disagree.  The documentation is not JUST to provide doctesting but also examples for users.  In this case, they might not know that e.g. 1000 (or 500, or 101) might provide enough more detail.  It's sort of saying our recommendation for more details.  That said, 200 or 150 should be fine for testing.  But it should be bigger! 

[Ralf Stephan]

What's wrong with
"Set plot_points to a higher value to get more detail."

--
You received this message because you are subscribed to a topic in the Google Groups "sage-devel" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sage-devel/k3kIRIk475A/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sage-devel+...@googlegroups.com.