Opened 11 years ago

Closed 10 years ago

#11899 closed enhancement (fixed)

Add #long time at various places

Reported by: jdemeyer Owned by: mvngu
Priority: minor Milestone: sage-4.8
Component: doctest coverage Keywords: sd35
Cc: leif Merged in: sage-4.8.alpha5
Authors: Jeroen Demeyer Reviewers: Julian Rueth
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by saraedum)

Throughout Sage, there are a few doctests taking a very long time (more than 30 seconds) but sometimes these can easily be simplified to take less time. Also add # long time where needed.


Apply

  1. 11899_2.patch

to the sage repository

Attachments (2)

11899.patch (23.0 KB) - added by jdemeyer 11 years ago.
11899_2.patch (25.5 KB) - added by saraedum 10 years ago.
rebased previous patch against 4.8.alpha3

Download all attachments as: .zip

Change History (17)

comment:1 follow-up: Changed 11 years ago by leif

  • Cc leif added

And on some machines the PExpect interfaces are very slow, or frequently hang... (There's some ticket by Simon King alleviating this in a couple of cases; don't recall the ticket number right now.)

Slightly related: We should IMHO make it such that the timeouts for doctests (perhaps optionally, or in addition) do not refer to wall but actual CPU time, since often doctests timeout just because they don't get enough CPU when doctesting with (more or less) many threads [and] on an already loaded machine.

comment:2 in reply to: ↑ 1 ; follow-up: Changed 11 years ago by jdemeyer

Replying to leif:

Slightly related: We should IMHO make it such that the timeouts for doctests (perhaps optionally, or in addition) do not refer to wall but actual CPU time, since often doctests timeout just because they don't get enough CPU when doctesting with (more or less) many threads [and] on an already loaded machine.

In principle I agree, but I don't think that is implementable in a good and portable way.

comment:3 Changed 11 years ago by leif

P.S.: If you want to see which line of a doctest takes how long, you can

$ cd $SAGE_ROOT/local/bin
$ mv ncadoctest.py ncadoctest.py.orig
$ cp -p sage:/home/leif/Sage/scripts/ncadoctest-v0.3.py ncadoctest.py

and test the file(s) in verbose mode.

There's no patch worth to get merged yet; I'll one dayTM develop this further and implement this behaviour as a separate option to sage -t ....

Note that with this version of ncadoctest.py the output when a doctest fails in non-verbose mode also differs.

comment:4 Changed 11 years ago by leif

s/cp/scp/

comment:5 in reply to: ↑ 2 ; follow-up: Changed 11 years ago by leif

Replying to jdemeyer:

Replying to leif:

We should IMHO make it such that the timeouts for doctests (perhaps optionally, or in addition) do not refer to wall but actual CPU time [...]

In principle I agree, but I don't think that is implementable in a good and portable way.

We could use setrlimit() in sage-doctest.py (which tests just a single file, i.e., each doctested file has its own instance), or add appropriate code (e.g. setting SIGALRM, and checking the consumed CPU time with getrusage()) to the generated temporary files with the examples that get run by Python. Emitting such extra code could be optional, too.

As a first step, we could in addition print the CPU time a doctest (of a complete file) took, by using getrusage() in sage-test.py / sage-ptest.py

comment:6 in reply to: ↑ 5 Changed 11 years ago by jdemeyer

Replying to leif:

We could use setrlimit() in sage-doctest.py

This only works for one process, so it will not work well with pexpect interfaces which spawn several child processes.

comment:7 Changed 11 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 11 years ago by jdemeyer

  • Authors set to Jeroen Demeyer

Most of these timings seem to be regressions, see sage-devel.

comment:9 Changed 11 years ago by jdemeyer

  • Summary changed from Simplify some very long doctests to Add #long time at various places

Changed 11 years ago by jdemeyer

comment:10 Changed 11 years ago by jdemeyer

  • Status changed from new to needs_review

comment:11 Changed 11 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

Changed 10 years ago by saraedum

rebased previous patch against 4.8.alpha3

comment:12 Changed 10 years ago by saraedum

  • Description modified (diff)
  • Milestone set to sage-4.8
  • Reviewers set to Julian Rueth

Apply only 11899_2.patch

(will set this to positive review when the doctests have finished)

comment:13 Changed 10 years ago by saraedum

  • Status changed from needs_review to positive_review

comment:14 Changed 10 years ago by saraedum

  • Keywords sd35 added

comment:15 Changed 10 years ago by jdemeyer

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