Opened 5 years ago

Closed 5 years ago

#23284 closed defect (fixed)

Testing padic_base_leaves.py takes a VERY long time

Reported by: Jeroen Demeyer Owned by:
Priority: major Milestone: sage-8.0
Component: padics Keywords:
Cc: David Roe, Julian Rüth Merged in:
Authors: David Roe Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 748b68c (Commits, GitHub, GitLab) Commit: 748b68cc2154ebc8cd77df63e491074f9be4a2ab
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

On a particular server, doing the long doctests of src/sage/rings/padics/padic_base_leaves.py:

  • Sage 8.0.beta9: [142 tests, 202.82 s]
  • Sage 8.0.beta10: [191 tests, 626.31 s]

The test time in Sage 8.0.beta10 is simply too much (but the number of tests has increased since 8.0.beta9, so this is probably not a regression). This is a problem because it leads to timeouts when running doctests.

Change History (28)

comment:1 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 5 years ago by Frédéric Chapoton

For me, on a Dell Optiplex 9020.

lINE 242: Test ran for 65.62 s
LINE 243 : Test ran for 105.39 s

[191 tests, 304.13 s]

With many slow TestSuite(R).run()

EDIT forgot to say, on 8.0.b11

Last edited 5 years ago by Frédéric Chapoton (previous) (diff)

comment:3 in reply to:  2 Changed 5 years ago by Jeroen Demeyer

The main thing to check is whether this is a regression or not. I plan to test some older versions of Sage.

comment:4 Changed 5 years ago by Travis Scrimshaw

Could there be something in there which calls the optional spkg normaliz? It does not like to be run in parallel because of its own parallelization. I ran across this and have a post about it on sage-devel.

comment:5 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

comment:6 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

comment:7 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

comment:8 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

comment:9 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

comment:10 Changed 5 years ago by David Roe

I understand where some of these changes are coming from. I'll take a look next week (no internet over the weekend).

comment:11 Changed 5 years ago by Karl-Dieter Crisman

Yikes, 343 seconds on my Mac laptop too (with nothing else going on).

comment:12 Changed 5 years ago by Jeroen Demeyer

Description: modified (diff)

I investigated the first regression and it is due to MPIR-3.0.0: #23327

I will change this ticket to be only about the second issue: the large absolute time taken by testing padic_base_leaves.py

comment:13 Changed 5 years ago by David Roe

Branch: u/roed/testing_padic_base_leaves_py_takes_a_very_long_time

comment:14 Changed 5 years ago by git

Commit: 8ce532e123d180e6bc5e0ad0647ec582684c49f1

Branch pushed to git repo; I updated commit sha1. New commits:

8ce532eMake long tests in padic_base_leaves.py shorter, add documentation for timings options to unit test code

comment:15 Changed 5 years ago by David Roe

Authors: David Roe
Status: newneeds_review

These changes drop the long tests from 256s to 37s.

comment:16 Changed 5 years ago by Jeroen Demeyer

How are the changes in src/sage/misc/sage_unittest.py related to this ticket? If they are not, please create a new ticket for that change.

comment:17 Changed 5 years ago by Jeroen Demeyer

Some more tests should be marked # long time, in particular on lines 244, 246, 336, 431.

comment:18 Changed 5 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

comment:19 Changed 5 years ago by Jeroen Demeyer

Why do you need separate runs of TestSuite here?

sage: TestSuite(R).run()
sage: TestSuite(R).run(elements = [R.random_element() for i in range(2^10)], max_runs = 2^12, skip='_test_metric') # long time
sage: R._test_metric(elements = [R.random_element() for i in range(2^3)]) # long time

Why is TestSuite(R).run() not sufficient?

(This is just a question, there might be a good reason)

comment:20 Changed 5 years ago by Karl-Dieter Crisman

Doctesting 1 file.
sage -t --long src/sage/rings/padics/padic_base_leaves.py
    [209 tests, 49.45 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 49.6 seconds
    cpu time: 44.5 seconds
    cumulative wall time: 49.5 seconds

comment:21 Changed 5 years ago by git

Commit: 8ce532e123d180e6bc5e0ad0647ec582684c49f1748b68cc2154ebc8cd77df63e491074f9be4a2ab

Branch pushed to git repo; I updated commit sha1. New commits:

1f0a79cModifying long tests in padic_base_leaves.py
748b68cUndo changes to sage_unittest.py

comment:22 in reply to:  16 Changed 5 years ago by David Roe

Replying to jdemeyer:

How are the changes in src/sage/misc/sage_unittest.py related to this ticket? If they are not, please create a new ticket for that change.

They got accidentally included. I've removed them.

comment:23 in reply to:  19 Changed 5 years ago by David Roe

Replying to jdemeyer:

Why do you need separate runs of TestSuite here?

sage: TestSuite(R).run()
sage: TestSuite(R).run(elements = [R.random_element() for i in range(2^10)], max_runs = 2^12, skip='_test_metric') # long time
sage: R._test_metric(elements = [R.random_element() for i in range(2^3)]) # long time

Why is TestSuite(R).run() not sufficient?

(This is just a question, there might be a good reason)

Just want tests with random elements, not just the ones returned by some_elements, which may not reveal problems.

comment:24 Changed 5 years ago by David Roe

Status: needs_workneeds_review

I've added a few # long time tags.

comment:25 Changed 5 years ago by Volker Braun

Priority: blockermajor

comment:26 Changed 5 years ago by Jeroen Demeyer

Reviewers: Jeroen Demeyer
Status: needs_reviewpositive_review

Blocker or not, it would be good to merge this quickly.

comment:27 in reply to:  16 Changed 5 years ago by David Roe

Replying to jdemeyer:

How are the changes in src/sage/misc/sage_unittest.py related to this ticket? If they are not, please create a new ticket for that change.

This is now #23335. I'd be interested in feedback on the interface there.

comment:28 Changed 5 years ago by Volker Braun

Branch: u/roed/testing_padic_base_leaves_py_takes_a_very_long_time748b68cc2154ebc8cd77df63e491074f9be4a2ab
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.