Do not fail when java is available but jmol is not.

[tornaria]

If java is not available, sagemath will fallback to tachyon gracefully.

When java is available, sagemath will assume jmol is available and error if not. This commit fixes the issue by implementing a method is_jmol_available() to replace is_jvm_available().

📝 Checklist

• The title is concise, informative, and self-explanatory.
• The description explains in detail what this PR is about.
• I have created tests covering the changes.

[mkoeppe]

Would you mind rewriting it to go through sage.features?

[tornaria]

Would you mind rewriting it to go through sage.features?

I don't see what problem would that solve.

[mkoeppe]

It could make the feature jmol available, suitable for conditionalizing doctests using # needs jmol.

[tornaria]

It could make the feature jmol available, suitable for conditionalizing doctests using # needs jmol.

And what problem would that solve?

[mkoeppe]

Path to #31027?

[tornaria]

Path to #31027?

Three years old and counting. In fact jmol is optional already. I don't have it installed and nothing ever failed for this reason, until today (something installed some java for some reason; sage still works, is only doctesting that fails).

I'd rather this PR be merged now (ideally for 10.2) and whoever wants can rework this as a feature for #31027. The check is very easy: if you run java -jar {JMOL_DIR}/JmolData.jar -i it will succeed if jmol is available and works, and fail if it isn't.

[mkoeppe]

I don't have time to test this at the moment, but you may want to look at what's going on in @tobiasdiez's conda workflow here

[tornaria]

I don't have time to test this at the moment, but you may want to look at what's going on in @tobiasdiez's conda workflow here

It seems just stdout or stderr output. I'll just silence the subprocess() call. My version of JmolData.jar claims -i silences it, and it does. Apparently conda version doesn't.

In fact, it's a bit overkill, just checking if the jar file exists is enough to solve my issue, so if the current version doesn't work I'll just do that.

[tornaria]

The failure is an unrelated timeout. Something very strange is happening, I was able to kind of reproduce this as follows:

$ sage -tp 16 --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py{,,,,,,,}{,}
too many failed tests, not using stored timings
Running doctests with ID 2023-11-26-21-39-35-a86f2520.
Running with SAGE_LOCAL='/usr' and SAGE_VENV='/usr'
Using --optional=pip,sage
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,cvxopt,cvxopt,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_cubic_hecke,database_jones_numfield,database_knotinfo,dvipng,fpylll,gap_package_atlasrep,gap_package_design,gap_package_grape,gap_package_guava,gap_package_hap,gap_package_polycyclic,gap_package_qpa,gap_package_quagroup,gfan,graphviz,imagemagick,ipython,jupymake,kenzo,latte_int,lrcalc_python,lrslib,mcqd,meataxe,mpmath,msolve,nauty,networkx,numpy,palp,pandoc,pdf2svg,pdftocairo,pexpect,phitigra,pillow,plantri,polytopes_db,polytopes_db_4d,pplpy,primecountpy,ptyprocess,pynormaliz,pyparsing,python_igraph,requests,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.libs.ecl,sage.libs.flint,sage.libs.gap,sage.libs.linbox,sage.libs.m4ri,sage.libs.ntl,sage.libs.pari,sage.libs.singular,sage.misc.cython,sage.modular,sage.modules,sage.numerical.mip,sage.plot,sage.rings.complex_double,sage.rings.finite_rings,sage.rings.function_field,sage.rings.number_field,sage.rings.padics,sage.rings.polynomial.pbori,sage.rings.real_double,sage.rings.real_mpfr,sage.sat,sage.schemes,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,scipy,singular,sphinx,sympy,tdlib
Doctesting 16 files using 16 threads.
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 21.60 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 26.92 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 30.49 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 29.89 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 30.73 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 33.24 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 34.18 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 33.80 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 35.24 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 35.11 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 41.31 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 42.23 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 42.29 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 48.41 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 48.93 s]
sage -t --long --random-seed=286735480429121101562228604801325644303 src/sage/rings/tests.py
    [62 tests, 212.75 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 215.0 seconds
    cpu time: 494.5 seconds
    cumulative wall time: 747.1 seconds
Features detected for doctesting: sage.modules,sage.rings.finite_rings,sage.rings.number_field,sage.rings.padics,sage.symbolic

Box is 8 core / 8 thread, otherwise unloaded. I did 16 threads to stress it. Testing this file just once takes ~16 seconds so he first 15 jobs are "kind of" reasonable. The last one is not, and is close to the 300s timeout. I fixed the seed, but maybe this test has another random not controlled by seed and is very slow sometimes.

I think this supports doubling the "hard" timeout as in #36223 (hi @orlitzky), but also having a soft timeout that will keep testing BUT COMPLAIN LOUDLY (not just a warning). And maybe making the soft timeout smaller in general except a a few specific files can increase the soft timeout. The point is, for a file that normally takes 16s, it is very bad that sometimes takes > 300s.

I was about to give compliments for CI getting better, and then this... 🤣

[tornaria]

Quoting #36031 (comment):

Marking this as a blocker in the hope it can sneak into 10.2 with the other open blocker

[tornaria]

This should be an easy review. All this is doing is, in addition to check java is available, also check the jmol jar file is available using os.path.isfile(self.jmolpath()).

[orlitzky]

I agree with the change but we're dropping the cygwin path handling in #36779 and this will conflict (you were here first, but the other one is already positively reviewed).

Having it go through sage.features might be nice for #31027 but this is a bugfix as-is.

[tornaria]

I couldn't care less about either sage.features or cygwin.

I agree with the change but we're dropping the cygwin path handling in #36779 and this will conflict (you were here first, but the other one is already positively reviewed).

Having it go through sage.features might be nice for #31027 but this is a bugfix as-is.

Just merge this PR in #36779.

[orlitzky]

Just merge this PR in #36779.

If it weren't already positively-reviewed, I would, but I prefer not to annoy innocent bystanders by undoing their already-completed work.

[tornaria]

Just merge this PR in #36779.

If it weren't already positively-reviewed, I would, but I prefer not to annoy innocent bystanders by undoing their already-completed work.

There, I fixed that for you.

[github-actions]

Documentation preview for this PR (built with commit c7670d0; changes) is ready! 🎉

[tornaria]

Not in 10.2, not in 10.3.beta0, not in 10.3.beta1. Is priority "blocker" good for anything?

[mkoeppe]

Is priority "blocker" good for anything?

I think the only time when Volker's script takes the priority label into consideration is during the release candidate phase. Otherwise it's done in numerical order by PR numbers. That's certainly order-theoretically sound.