Opened 5 years ago

Closed 5 years ago

#23865 closed defect (fixed)

disable a very long doctest in omega.py

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.1
Component: documentation Keywords:
Cc: dkrenn Merged in:
Authors: Daniel Krenn Reviewers: Daniel Krenn, Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: 12734e4 (Commits, GitHub, GitLab) Commit: 12734e4b684ac9eac6f7484169bb7c7e37ba5d62
Dependencies: Stopgaps:

Status badges

Description

Disable the following from sage/rings/polynomial/omega.py that takes forever without any justification

sage: from sage.rings.polynomial.omega import Omega_ge
sage: %time Omega_ge(0, (2, 2, 1, 1, 1, 1, 1, -1, -1))[0].number_of_terms()
CPU times: user 53.1 s, sys: 66 ms, total: 53.2 s
Wall time: 53.1 s
27837

(running the doctests in omega.py takes longer than running the doctests in all other files from sage/rings/polynomial/)

Change History (12)

comment:1 Changed 5 years ago by vdelecroix

  • Branch set to u/vdelecroix/23865
  • Commit set to f62bf2772454a0af69e5ed2723070c5936ecfdc8
  • Status changed from new to needs_review

New commits:

f62bf2723865: disable doctest in omega

comment:2 follow-up: Changed 5 years ago by jdemeyer

Instead of disabling it, could the test be simplified?

comment:3 Changed 5 years ago by vdelecroix

This is why I put dkrenn in copy! I am not exactly sure what this doctest is meant for.

comment:4 in reply to: ↑ 2 Changed 5 years ago by dkrenn

Replying to jdemeyer:

Instead of disabling it, could the test be simplified?

Yes, this can be simplified. I'll provide a corrected version in the next days.

comment:5 in reply to: ↑ description Changed 5 years ago by dkrenn

Replying to vdelecroix:

Disable the following from sage/rings/polynomial/omega.py that takes forever without any justification

The motivation for having this is, that there is a separate computation for the result meaning that the result is somehow verified. I do not recall that it took that long and will investigate.

comment:6 Changed 5 years ago by vdelecroix

Thanks Daniel!

comment:7 Changed 5 years ago by dkrenn

  • Branch changed from u/vdelecroix/23865 to u/dkrenn/23865

comment:8 Changed 5 years ago by dkrenn

  • Authors changed from Vincent Delecroix to Vincent Delecroix, Daniel Krenn
  • Commit changed from f62bf2772454a0af69e5ed2723070c5936ecfdc8 to 12734e4b684ac9eac6f7484169bb7c7e37ba5d62
  • Reviewers set to Daniel Krenn

Small reviewer patch added (untested --> not tested) and I've added a new, shorter doctest which needs a small cross review. [Good to go from my side.]


New commits:

1401a2423865: untested --> not tested
12734e423865: add one long time doctest

comment:9 Changed 5 years ago by dkrenn

FYI, all non-long tests in the file now take 1 second; including the long tests 2 seconds.

comment:10 Changed 5 years ago by vdelecroix

  • Authors changed from Vincent Delecroix, Daniel Krenn to Daniel Krenn
  • Reviewers changed from Daniel Krenn to Daniel Krenn, Vincent Delecroix

Indeed, on the patchbot it is now

sage -t --long src/sage/rings/polynomial/omega.py
    [127 tests, 1.84 s]

Thanks!

comment:11 Changed 5 years ago by vdelecroix

  • Status changed from needs_review to positive_review

oups. I forgot to positive review it...

comment:12 Changed 5 years ago by vbraun

  • Branch changed from u/dkrenn/23865 to 12734e4b684ac9eac6f7484169bb7c7e37ba5d62
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.