Replace second_on_modern_computer() with a fixed amount of work #33022
Comments
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Oct 26, 2024
Finally address sagemath#32981. Using wall time to measure which tests are "long" doesn't make much sense. Wall time is subject to every conflating factor: 1. Faster hardware runs tests in less wall time 2. Faster compiler flags makes code that runs in less wall time 3. A busy machine will run tests slower than an unburdened one 4. Operating system priority can increase or decrease the time taken by a process 5. etc. Ideally, everyone would agree on how long is too long for a test to run. Attaining that ideal is not so easy, but the first step is to switch`--warn-long` away from using wall time and towards using CPU time. A second phase (sagemath#33022) will normalize the CPU time. ### Examples #### --warn-long output The user-facing portion of this can be tested with a low `--warn-long` value, ``` $ sage -t --warn-long=1 src/sage/rings/qqbar.py ... Doctesting 1 file. sage -t --warn-long --random- seed=309742757431591512726158538133919135994 src/sage/rings/qqbar.py ********************************************************************** File "src/sage/rings/qqbar.py", line 511, in sage.rings.qqbar Warning, slow doctest: z, = [r for r in p4.roots(QQbar, False) if r in ival] Test ran for 1.51s cpu, 1.51s wall Check ran for 0.00s cpu, 0.00s wall ********************************************************************** File "src/sage/rings/qqbar.py", line 524, in sage.rings.qqbar Warning, slow doctest: z2 = QQbar.polynomial_root(p4, ival) Test ran for 1.07s cpu, 1.07s wall Check ran for 0.00s cpu, 0.00s wall ********************************************************************** File "src/sage/rings/qqbar.py", line 808, in sage.rings.qqbar.AlgebraicField_common._factor_multivariate_polynomial Warning, slow doctest: F = QQbar._factor_multivariate_polynomial(p) Test ran for 2.38s cpu, 2.42s wall Check ran for 0.00s cpu, 0.00s wall ... ``` #### pexpect accounting Two different methods are used to account for the CPU time, one for linux, and one for everything else. This GAP command uses pexpect and should chew up a good bit of CPU time: ``` sage: from sage.doctest.util import Timer sage: t = Timer() sage: _ = t.start(); t.stop() # no CPU time used between start/stop {'cputime': 0.0, 'walltime': 7.605552673339844e-05} sage: _ = t.start(); gap.eval('a:=List([0..10000],i->WordAlp("a",i));; IsSortedList(a);'); t.stop() # lots of CPU time used 'true' {'cputime': 30.089999999999996, 'walltime': 36.292088747024536} ``` URL: sagemath#36226 Reported by: Michael Orlitzky Reviewer(s): Gonzalo Tornaría, John H. Palmieri, Michael Orlitzky, Tobias Diez
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In #32973, #32981, and #32995 some improvements to the
--warn-longfeature are discussed. Particularly in #32981, thesecond_on_modern_computer()multiplier is dropped, because as it stands, it often disables the--warn-longwarnings entirely.To reinstate the "CPU handicap," we should replace that function with one that performs a fixed amount of work. The amount of CPU time taken to perform that work can then be used as a multiplier to tweak the default
--warn-longvalues.This idea is still not perfect, but it should be better than a constant value, and avoids the problems with using old doctest statistics.
CC: @tornaria
Component: doctest framework
Issue created by migration from https://trac.sagemath.org/ticket/33022
The text was updated successfully, but these errors were encountered: