Opened 6 years ago

Closed 6 years ago

#23284 closed defect (fixed)

Testing padic_base_leaves.py takes a VERY long time

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

GitHub link to the corresponding issue

Description (last modified by jdemeyer)

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 6 years ago by jdemeyer

Description: modified (diff)

comment:2 Changed 6 years ago by 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 6 years ago by chapoton (previous) (diff)

comment:3 in reply to:  2 Changed 6 years ago by jdemeyer

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 6 years ago by tscrim

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 6 years ago by jdemeyer

Description: modified (diff)

comment:6 Changed 6 years ago by jdemeyer

Description: modified (diff)

comment:7 Changed 6 years ago by jdemeyer

Description: modified (diff)

comment:8 Changed 6 years ago by jdemeyer

Description: modified (diff)

comment:9 Changed 6 years ago by jdemeyer

Description: modified (diff)

comment:10 Changed 6 years ago by roed

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 6 years ago by kcrisman

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

comment:12 Changed 6 years ago by jdemeyer

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 6 years ago by roed

Branch: u/roed/testing_padic_base_leaves_py_takes_a_very_long_time

comment:14 Changed 6 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 6 years ago by roed

Authors: David Roe
Status: newneeds_review

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

comment:16 Changed 6 years ago by 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.

comment:17 Changed 6 years ago by jdemeyer

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

comment:18 Changed 6 years ago by jdemeyer

Status: needs_reviewneeds_work

comment:19 Changed 6 years ago by 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)

comment:20 Changed 6 years ago by kcrisman

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 6 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 6 years ago by roed

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 6 years ago by roed

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 6 years ago by roed

Status: needs_workneeds_review

I've added a few # long time tags.

comment:25 Changed 6 years ago by vbraun

Priority: blockermajor

comment:26 Changed 6 years ago by jdemeyer

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 6 years ago by roed

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 6 years ago by vbraun

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.