#28819 closed defect (fixed)

disable problematic external doctests

Reported by: gh-mwageringel Owned by:
Priority: major Milestone: sage-9.0
Component: doctest framework Keywords: matlab, mathematica
Cc: Merged in:
Authors: Markus Wageringel Reviewers: Kwankyu Lee
Report Upstream: N/A Work issues:
Branch: 2b5583e (Commits, GitHub, GitLab) Commit: 2b5583eb59741e194bb06cdf84301605f6111276
Dependencies: Stopgaps:

Status badges

Description

This ticket marks the doctests in sage.doctest.external involving the standard interfaces to external software as not tested and some tests in the Mathematica interface as mathematicafrontend.

In particular, this is a workaround for the following problem:

Whenever Sage runs the tests for src/sage/doctest/external.py, it tries to run a computation in Matlab. Even though the interface sort of works in interactive mode, it does not work at all during doctest mode, causing Matlab to crash which results in two numbered log files being written to the user home directory such as matlab_crash_dump.281119-1 and java.log.18975. When running a patchbot, the home directory gets flooded by these files.

Regardless of the Matlab issue, external software like this should not be invoked during normal test runs, unless explicitly enabled when running all or external tests. Especially this applies to software that requires a license to run.

The doctests this ticket disables in sage.doctest.external invoke functions that catch all exceptions and produce random output, so there is not much value in running those tests at all.

The Matlab problem still persists when running external doctests, but in that case failures are to be expected.


A secondary problem is solved in the Mathematica interface:

Usually, the Mathematica interface interacts with the program WolframKernel. However, a few doctests related to rendering result in the frontend program Mathematica to be launched by the WolframKernel. This is because graphics cannot be exported without access to a frontend, according to this question.

These instances of Mathematica do not always seem to get closed properly. When running a patchbot, this leads to a growing number of orphan processes of Mathematica which do not seem to react to a SIGTERM signal.

Marking the doctests as mathematicafrontend effectively disables them. This solution had already been applied by #23112 for one of the doctests in the file.

Change History (15)

comment:1 Changed 17 months ago by gh-mwageringel

  • Authors set to Markus Wageringel
  • Branch set to u/gh-mwageringel/28819
  • Commit set to 8584c39a07107502d5a4376f515152821effd644

New commits:

fc0ebd128819: avoid running mathematica frontend in doctests
8584c3928819: avoid running external software during normal test runs

comment:2 Changed 17 months ago by gh-mwageringel

  • Keywords matlab mathematica added
  • Status changed from new to needs_review

I let the patchbot run on this ticket a couple of times and it works as intended.

comment:3 Changed 17 months ago by gh-mwageringel

  • Milestone changed from sage-8.9 to sage-9.0

comment:4 follow-up: Changed 17 months ago by klee

Regardless of the Matlab issue, external software like this should not be invoked during normal test runs, unless explicitly enabled when running all > or external tests. Especially this applies to software that requires a license to run.

Then.., isn't this a right solution for the problems?

        sage: from sage.doctest.external import has_matlab
        sage: has_matlab()  # optional -- matlab
        True  

comment:5 Changed 17 months ago by git

  • Commit changed from 8584c39a07107502d5a4376f515152821effd644 to 304b51e4f202a9cd68c86998d8938bf493927391

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

304b51e28819: avoid running external software during normal test runs

comment:6 in reply to: ↑ 4 Changed 17 months ago by gh-mwageringel

Replying to klee:

Then.., isn't this a right solution for the problems?

        sage: from sage.doctest.external import has_matlab
        sage: has_matlab()  # optional -- matlab
        True  

Indeed, that also works and has the advantage that the tests are not deactivated entirely. I have updated the branch accordingly and let the patchbot run on it. It works as intended on my end.

comment:7 Changed 17 months ago by klee

  • Reviewers set to Kwankyu Lee
  • Status changed from needs_review to positive_review

Thanks. It looks good to me.

comment:8 Changed 17 months ago by gh-mwageringel

Thank you for the review.

comment:9 Changed 17 months ago by klee

  • Status changed from positive_review to needs_work

comment:10 follow-up: Changed 17 months ago by klee

I now realized that the tag should be

sage: has_matlab()  # random, optional -- matlab
True

because the doctest may fail when we do

sage -bt --optional=sage,optional,matlab src/sage/doctest/external.py 

comment:11 in reply to: ↑ 10 ; follow-up: Changed 17 months ago by gh-mwageringel

Replying to klee:

because the doctest may fail when we do

sage -bt --optional=sage,optional,matlab src/sage/doctest/external.py 

Well, that is just because the Matlab interface is broken. Running the Matlab tests on src/sage/interfaces/matlab.py fails for the same reason, currently. When the interface is fixed, the function has_matlab() should always return True – otherwise the doctest would reveal an actual bug in has_matlab().

Until then, I think it is fine if the doctest fails when the optional matlab flag is set, as it correctly indicates a problem. At this point, nobody seems to care much about the Matlab interface nor run its tests, anyway.

comment:12 in reply to: ↑ 11 ; follow-up: Changed 17 months ago by klee

When the interface is fixed, the function has_matlab() should always return True – otherwise the doctest would reveal an actual bug in has_matlab().

No. If you don't have matlab, then it should return False. Not only for matlab. Let's say you don't have magma, then

sage: has_magma()  # optional -- magma
True

would fail if you run, for whatever reason,

sage -bt --optional=sage,optional,magma src/sage/doctest/external.py 

comment:13 in reply to: ↑ 12 ; follow-up: Changed 17 months ago by gh-mwageringel

  • Branch changed from u/gh-mwageringel/28819 to u/gh-mwageringel/28819v2
  • Commit changed from 304b51e4f202a9cd68c86998d8938bf493927391 to 2b5583eb59741e194bb06cdf84301605f6111276
  • Status changed from needs_work to needs_review

Replying to klee:

Let's say you don't have magma, then

sage: has_magma()  # optional -- magma
True

would fail

Every doctest in Sage tagged optional -- magma will fail if one does not have Magma, so this is expected. I do not have a strong opinion about this though, as it still solves the problem reported in this ticket, so here is a branch where the doctests in question are marked random.


New commits:

2b5583e28819: avoid running external software during normal test runs

comment:14 in reply to: ↑ 13 Changed 17 months ago by klee

  • Status changed from needs_review to positive_review

Replying to gh-mwageringel:

Replying to klee:

Let's say you don't have magma, then

sage: has_magma()  # optional -- magma
True

would fail

Every doctest in Sage tagged optional -- magma will fail if one does not have Magma

But unlike every other doctest with optional -- magma, by its nature, has_magma() should work without Magma. I think optional -- magma is used here simply as a trick to turn it off unless one explicitly wants to test it.

I do not have a strong opinion about this though, as it still solves the problem reported in this ticket, so here is a branch where the doctests in question are marked random.

Thanks.

comment:15 Changed 17 months ago by vbraun

  • Branch changed from u/gh-mwageringel/28819v2 to 2b5583eb59741e194bb06cdf84301605f6111276
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.