Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make: docbuilding should depend on jmol #21014

Closed
jhpalmieri opened this issue Jul 13, 2016 · 37 comments
Closed

make: docbuilding should depend on jmol #21014

jhpalmieri opened this issue Jul 13, 2016 · 37 comments

Comments

@jhpalmieri
Copy link
Member

[plot3d   ] /Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-7.3.beta7/local/lib/python2.7/site-packages/sage/plot/plot3d/platonic.py:docstring of sage.plot.plot3d.platonic:10: WARNING: Exception occurred in plotting platonic-1
[plot3d   ] from /Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-7.3.beta7/src/doc/en/reference/plot3d/sage/plot/plot3d/platonic.rst:
[plot3d   ] Traceback (most recent call last):
[plot3d   ] File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-7.3.beta7/local/lib/python2.7/site-packages/matplotlib-1.5.1-py2.7-macosx-10.9-x86_64.egg/matplotlib/sphinxext/plot_directive.py", line 517, in run_code
[plot3d   ] six.exec_(code, ns)
[plot3d   ] File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-7.3.beta7/local/lib/python2.7/site-packages/matplotlib-1.5.1-py2.7-macosx-10.9-x86_64.egg/matplotlib/externals/six.py", line 672, in exec_
[plot3d   ] exec("""exec _code_ in _globs_, _locs_""")
[plot3d   ] File "<string>", line 1, in <module>
[plot3d   ] File "<string>", line 4, in <module>
[plot3d   ] File "<string>", line 8, in sphinx_plot
[plot3d   ] File "sage/plot/plot3d/base.pyx", line 1515, in sage.plot.plot3d.base.Graphics3d.save (build/cythonized/sage/plot/plot3d/base.c:17559)
[plot3d   ] self.save_image(filename)
[plot3d   ] File "sage/plot/plot3d/base.pyx", line 1444, in sage.plot.plot3d.base.Graphics3d.save_image (build/cythonized/sage/plot/plot3d/base.c:17101)
[plot3d   ] self._save_image_png(filename, **kwds)
[plot3d   ] File "sage/plot/plot3d/base.pyx", line 1408, in sage.plot.plot3d.base.Graphics3d._save_image_png (build/cythonized/sage/plot/plot3d/base.c:16685)
[plot3d   ] scene = self._rich_repr_jmol(**opts)
[plot3d   ] File "sage/plot/plot3d/base.pyx", line 269, in sage.plot.plot3d.base.Graphics3d._rich_repr_jmol (build/cythonized/sage/plot/plot3d/base.c:6520)
[plot3d   ] jdata.export_image(targetfile=preview_png, datafile=script,
[plot3d   ] File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-7.3.beta7/local/lib/python2.7/site-packages/sage/interfaces/jmoldata.py", line 181, in export_image
[plot3d   ] raise RuntimeError("Jmol failed to create file %s, see %s for details"%(repr(targetfile), repr(scratchout)))
[plot3d   ] RuntimeError: Jmol failed to create file '/Users/palmieri/.sage/temp/Johns-iMac.local/28162/dir_7BaenG/preview.png', see '/Users/palmieri/.sage/temp/Johns-iMac.local/28162/tmp_zR2J1Q.txt' for details

See https://groups.google.com/d/msg/sage-devel/SlpXXtK_Cy0/QynwDq2gAAAJ and https://groups.google.com/d/msg/sage-release/jQmE2nsnbDI/u32jH0vOAQAJ.

Component: build

Author: John Palmieri

Branch/Commit: d741fbb

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/21014

@jhpalmieri jhpalmieri added this to the sage-7.3 milestone Jul 13, 2016
@jhpalmieri
Copy link
Member Author

@jhpalmieri
Copy link
Member Author

Commit: d741fbb

@jhpalmieri
Copy link
Member Author

New commits:

d741fbbWhen running make, the doc build should depend on jmol.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Jul 13, 2016

comment:4

If we make plots then we run arbitrary Sage code. So all runtime dependencies are docbuild dependencies, no? And if not then somebody just hasn't make a sufficiently complicated plot yet. What I'm proposing thus is to just use a dependency like

doc-html: all-build

@dimpase
Copy link
Member

dimpase commented Jul 13, 2016

comment:5

indeed, trouble probably started few months ago when people did few tickets putting nicer pictures into the docs.

@jdemeyer
Copy link

comment:6

Replying to @vbraun:

If we make plots then we run arbitrary Sage code. So all runtime dependencies are docbuild dependencies, no? And if not then somebody just hasn't make a sufficiently complicated plot yet. What I'm proposing thus is to just use a dependency like

doc-html: all-build

This used to be the case a while ago, but it makes the parallel build less efficient. For example, currently R can be built in parallel with the docs. With your proposal, parallel builds would take more time. I'm not saying that this is a huge problem, just pointing it out.

@jhpalmieri
Copy link
Member Author

comment:7

We could add sagelib to the docbuilding dependencies.

@vbraun
Copy link
Member

vbraun commented Jul 13, 2016

comment:8

Replying to @jdemeyer:

This used to be the case a while ago, but it makes the parallel build less efficient. For example, currently R can be built in parallel with the docs

Yes, until somebody adds an R plot to the docs. Then suddenly you get hard-to-debug parallel build failures.

@jhpalmieri
Copy link
Member Author

comment:9

Replying to @jhpalmieri:

We could add sagelib to the docbuilding dependencies.

Sorry, it's already there. Never mind.

@dimpase
Copy link
Member

dimpase commented Jul 13, 2016

comment:10

Replying to @jdemeyer:

Replying to @vbraun:

If we make plots then we run arbitrary Sage code. So all runtime dependencies are docbuild dependencies, no? And if not then somebody just hasn't make a sufficiently complicated plot yet. What I'm proposing thus is to just use a dependency like

doc-html: all-build

This used to be the case a while ago, but it makes the parallel build less efficient. For example, currently R can be built in parallel with the docs.

but this is only true as long as there is no doctest using R somewhere you do not expect it to be used.

With your proposal, parallel builds would take more time. I'm not saying that this is a huge problem, just pointing it out.

@jdemeyer
Copy link

comment:11

Replying to @vbraun:

Then suddenly you get hard-to-debug parallel build failures.

You could easily add a buildbot testing make distclean; make doc.

@jdemeyer
Copy link

comment:12

Sorry, but this issue worksforme. What is the exact issue that we're trying to solve here? The ticket description should be more explicit.

@dimpase

This comment has been minimized.

@jdemeyer
Copy link

comment:14

That's not very explicit. What exactly is the problem since I cannot reproduce it?

@jdemeyer
Copy link

comment:15

Also: it's not a problem if doctests use jmol, this is only about docbuilding.

@dimpase
Copy link
Member

dimpase commented Jul 14, 2016

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Jul 14, 2016

comment:16

I don't think that the fact that not everybody can reproduce this (race conditions are not easy in this sense) should postpone this ticket.

@jhpalmieri
Copy link
Member Author

comment:17

The issue is that some plots in the documentation require jmol, so if jmol happens not to be built early enough, docbuilding can fail. I just saw this yesterday on a fresh copy of 7.3.beta7, and there are other reports in sage-devel and sage-release.

@jdemeyer
Copy link

comment:18

Replying to @jhpalmieri:

The issue is that some plots in the documentation require jmol, so if jmol happens not to be built early enough, docbuilding can fail.

I understand that, but nobody has been explicit about which plot is the one which is supposed to fail without jmol. And again, I cannot reproduce the problem: for me, the doc builds without jmol.

I really don't like to add a "fix" for a problem when it's not at all clear what the problem is and whether the "fix" fixes it.

I feel that the problem is more subtle and we're missing something here...

@jhpalmieri
Copy link
Member Author

comment:19

It fails for me. Here's what I did: I started with a fresh tarball, ran make jmol and then deleted local/bin/jmol and local/share/jmol/. Then I ran make. Docbuilding now fails every time, with the error as in the attached log file (extracted from dochtml.log).

@jhpalmieri
Copy link
Member Author

Attachment: plot.log

@strogdon
Copy link

comment:20

There are numerous .txt files under .sage/temp/... that probably contain something like

Error: Unable to access jarfile $SAGE_ROOT/local/share/jmol/JmolData.jar

which is to be expected. I wonder what is in these .txt files for those that get failures when jmol is installed.

@vbraun
Copy link
Member

vbraun commented Jul 14, 2016

comment:21

I did a make distclean && make doc-html of plain 7.3.beta7 and it failed as expected with

[dochtml] [plot3d   ] RuntimeError: Jmol failed to create file '/home/vbraun/.sage/temp/volker-desktop/31502/dir_EhlRBl/preview.png', see '/home/vbraun/.sage/temp/volker-desktop/31502/tmp_tX4EGj.txt' for details

I'm pretty sure thats 100% reproducable, though takes a long time.

@jdemeyer
Copy link

comment:22

Replying to @vbraun:

I'm pretty sure thats 100% reproducable

I'm pretty sure thats not the case.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

comment:23

I agree with strogdon, it would be good to see also the .txt files that the error messages refer to.

@vbraun
Copy link
Member

vbraun commented Jul 15, 2016

comment:24

The txt files are auto-deleted by the sage cleaner. In any case I can tell you whats wrong, jmol is not a dependency of make doc-html so it is not built:

$ ./sage -sh -c jmol
/bin/bash: jmol: command not found

@jhpalmieri
Copy link
Member Author

comment:25

Those .txt files say:

Error: Unable to access jarfile /Users/palmieri/Desktop/Sage_stuff/sage_builds/clean/sage-7.3.beta7/local/share/jmol/JmolData.jar

@jdemeyer
Copy link

comment:26

Further analysis points to this if switch in src/sage/plot/plot3d/base.pyx in the _rich_repr_jmol() function:

        if not jdata.is_jvm_available():
            # We can only use JMol to generate preview if a jvm is installed
            from sage.repl.rich_output.output_graphics import OutputImagePng
            tachyon = self._rich_repr_tachyon(OutputImagePng, **opts)
            tachyon.png.save_as(preview_png)
        else:
            # Java needs absolute paths
            # On cygwin, they should be native ones
            scene_native = scene_zip
            import sys
            if sys.platform == 'cygwin':
                from subprocess import check_output, STDOUT
                scene_native = check_output(['cygpath', '-w', scene_native],
                                            stderr=STDOUT).rstrip()
            script = '''set defaultdirectory "{0}"\nscript SCRIPT\n'''.format(scene_native)
            jdata.export_image(targetfile=preview_png, datafile=script,
                               image_type="PNG",
                               figsize=opts['figsize'][0])

So the problem only occurs if Java is installed.

But this leaves the question: if Tachyon works, why not always use Tachyon to produce the plots?

@jdemeyer
Copy link

comment:27

To anybody which does have a working Java: does the interactive JMol applet actually work in the generated documentation? If not, there is no point in using JMol for those plots.

@vbraun
Copy link
Member

vbraun commented Jul 15, 2016

comment:28

We use jmol (and java) to build the png preview of the plot server-side. This is then displayed by the notebook until the 3-d plot is activated by clicking on it. The actual in-browser animation is via jsmol (javascript).

@dimpase
Copy link
Member

dimpase commented Jul 16, 2016

comment:29

besides, the quality of plots produced by Tachyon is not as good as with jmol...

@dimpase
Copy link
Member

dimpase commented Jul 16, 2016

tachyon output

@dimpase
Copy link
Member

dimpase commented Jul 16, 2016

Attachment: nose.png

Attachment: nose_jmol.png

jmol output (smoother, no artefacts...)

@jdemeyer
Copy link

Changed reviewer from Jeroen Demeyer, Dima Pasechnik to Dima Pasechnik

@vbraun
Copy link
Member

vbraun commented Jul 17, 2016

Changed branch from u/jhpalmieri/doc-build-depends-on-jmol to d741fbb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants