Opened 8 years ago

Closed 7 years ago

#12299 closed enhancement (fixed)

Upgrade Jmol to 12.3.27, Advance Jmol Interactive Features in Flask Notebook

Reported by: gutow Owned by: jason, mpatel, was
Priority: major Milestone: sage-5.4
Component: notebook Keywords: Jmol, 3D, notebook
Cc: novoselt, strogdon, ppurka, kini Merged in: sage-5.4.beta0
Authors: Jonathan Gutow Reviewers: Karl-Dieter Crisman, Steven Trogdon, Punarbasu Purkayastha, John Palmieri, Dan Drake
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: Commit:
Dependencies: #11080,#11078,#11503,#13121 Stopgaps:

Description (last modified by gutow)

This ticket provides the following advanced interactive features to Jmol in the notebook:

  1. Initial display is a static image created by Jmol (if a javaVM is available on the server) or Tachyon. This means all browsers and devices will at least be able to see a 3-D image.
  2. The live applet can be loaded by clicking on a "Make Interactive..." button.
  3. Colors can be changed without recalculating
  4. spin on checkbox
  5. high quality checkbox
  6. Meshes can be displayed without recalculating (no sparkles either :))
  7. Transparency can be adjusted without recalculating
  8. Applet size can be adjusted
  9. Applet can be displayed in its own window
  10. Can download a file that the Jmol application can open to recreate the exact view shown on the web page. This allows for more sophisticated manipulation and the semi-automatic creation of web pages that include this live 3-D image.
  11. Solves memory problems associated with opening too many Jmols at once, by automatically putting Jmols that are not being used to sleep.

Explicit Installation Instructions for <=sage-5.1:

  1. Install the flask notebook by following the instructions at #11080.
  2. Then do the following to apply the patches and install an updated version of Jmol.
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299-all-in-one.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299_headless_java.patch
./sage --hg -R devel/sage qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac-12299_java_doctest_opt.patch

./sage --hg -R devel/sagenb qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299_adv_jmol_nb.patch
./sage --hg -R devel/sagenb qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299_old_wksht.patch
./sage --hg -R devel/sagenb qimport -P http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299_server_sync_and_UI.2.patch

./sage -f  "http://www.uwosh.edu/faculty_staff/gutow/jmol-12.3.27.p2.spkg"

./sage -b

Explicit Installation Instructions for >sage-5.1:

  1. Install sagenb 0.10.x by following the instructions at #13121 - those instructions include everything necessary to test this ticket.

Notes:

If you are doing this in a different way you need to:

Attachments (14)

trac_12299_plot_3D_static_img.patch (9.5 KB) - added by gutow 7 years ago.
update to remove duplicate patches
trac_12299_adv_jmol_nb.patch (62.3 KB) - added by gutow 7 years ago.
update to remove duplicate patches
trac_12299_old_wksht.patch (3.4 KB) - added by gutow 7 years ago.
fix Jmol errors on loading pre-flask worksheets
trac_12299_server_sync_and_UI.patch (5.9 KB) - added by gutow 7 years ago.
Fix server<->Jmol syncing and tweak UI (no more sparkles and better vocabulary).
trac_12299_server_sync_and_UI.2.patch (5.9 KB) - added by gutow 7 years ago.
Fix server<->Jmol syncing and tweak UI (no more sparkles and better vocabulary).
trac_12299_jmoldata_doctests.patch (2.4 KB) - added by gutow 7 years ago.
complete doctest coverage in jmoldata.py and increase JmolData?.jar memory
trac_12299_jmoldata_doctests2.patch (4.6 KB) - added by gutow 7 years ago.
more doctests plus formatting fixes
trac_12299-reviewer.patch (7.6 KB) - added by kcrisman 7 years ago.
trac_12299_test_typos_JVM_mem.patch (1.5 KB) - added by gutow 7 years ago.
fix backslash issues and define maximum heap size for JVM
trac_12299-jhp.patch (2.4 KB) - added by jhpalmieri 7 years ago.
minor cleanup of paths in jmoldata.py
Screen shot 2012-06-18 at 8.28.42 AM.png (70.7 KB) - added by kcrisman 7 years ago.
Shows documentation not building properly
trac_12299-all-in-one.patch (10.8 KB) - added by jhpalmieri 7 years ago.
all Sage library patches, folded into one
trac_12299_headless_java.patch (972 bytes) - added by gutow 7 years ago.
reinserting headless option for java launch
trac-12299_java_doctest_opt.patch (1.0 KB) - added by gutow 7 years ago.
make java dependent doctests in jmoldata.py optional

Download all attachments as: .zip

Change History (214)

comment:1 Changed 8 years ago by gutow

  • Description modified (diff)

comment:2 Changed 8 years ago by gutow

  • Description modified (diff)

comment:3 Changed 8 years ago by gutow

  • Description modified (diff)
  • Status changed from new to needs_review

The second patch is very big, but I think it is best to do it as a patch to make sure that I haven't somehow rebased against the wrong versions of the files.

comment:4 Changed 8 years ago by novoselt

  • Cc novoselt added

comment:5 Changed 8 years ago by strogdon

  • Cc strogdon added

comment:6 Changed 8 years ago by gutow

Rebasing the patches to 5.0.beta6 and fixing issue of p0 versus p1 level patches.

comment:7 follow-up: Changed 8 years ago by strogdon

I've tried these patches on top 5.0.beta6 and #11080. I believe the patch to jmol_lib.js in trac_12299_adv_jmol_nb.patch appears twice. If I delete the first occurrence everything applies and the png images generated by Tachyon appear in the notebook. If I click the "Make Interactive" button then the 3-D jmol images appear. I noticed that the first image to always appear, even if a cell is reevaluated, is the png generated image. "Make Interactive" must be clicked to get the jmol image. If a jmol image is present in a cell, reevaluating that cell will display a png image. "Make Interactive" must be clicked again to recover the jmol image. If after clicking "Download this view" from the "Advanced Controls" and saving the jmol image there then appears in the lower right corner of the applet a "Jmol_S" the wasn't present initially. When clicked this gives a jmol menu. I believe this is part of jmol. I've seen it on sage-on-gentoo where jmol-12.0.45 is used. I'm not sure why it suddenly appears after the indicated action. This all looks very interesting.

comment:8 in reply to: ↑ 7 ; follow-up: Changed 8 years ago by gutow

Replying to strogdon: You are correct about the error in the adv_jmol_nb.patch. I have manually deleted the extra patch to jmol_lib.js and uploaded a correct version. I've been having trouble patching on top of #11080 patches. I believe I ran afoul of unfinished queues. Anyway, I think this correction is the last fix related to that.

Comments:

1) The red "Jmol_S" logo in the bottom right corner indicates that the signed applet has been loaded and has access to the user's directory tree. I believe I can turn it off, but think it is a bad idea. I want to be reminded that I am using an applet that can send stuff from my disk to the server. The menu is the same as the right-click menu that is always available with Jmol.

2) If we want to have an image as the default so that all devices will see something, users will have to deal with requesting it become interactive each time the cell is recalculated. The notebook+sage presently insists on reloading Jmol each time a recalculation is done. In order to do this properly we would need to have a separate div in the notebook for applets and not kill the applet unless it is not being used. Then Jmol would not revert to the starting over state each time a recalculation is done. I think that is another ticket for after these changes go in.

3) Would reloading with a static image be less of an issue if the .png was the same quality as the Jmol image? I'm working on generating static images using Jmol on the server if a Java Virtual Machine is installed on the server. Many details like graceful fallback, probably to Tachyon, are still being worked out.

Does this answer your questions or generate in suggestions for what needs work?

comment:9 in reply to: ↑ 8 Changed 8 years ago by strogdon

Replying to gutow:

Comments:

1) The red "Jmol_S" logo in the bottom right corner indicates that the signed applet has been loaded and has access to the user's directory tree. I believe I can turn it off, but think it is a bad idea. I want to be reminded that I am using an applet that can send stuff from my disk to the server. The menu is the same as the right-click menu that is always available with Jmol.

That explains things. For a sage-on-gentoo (s-o-g) build I had noticed from the jmol menu that the applet was signed but didn't associate this with the "_S". From the s-o-g install it would appear that the applets are always signed. I'm not sure it's possible for it to be otherwise. But that's a minor thing and not Sage related.

2) If we want to have an image as the default so that all devices will see something, users will have to deal with requesting it become interactive each time the cell is recalculated. The notebook+sage presently insists on reloading Jmol each time a recalculation is done. In order to do this properly we would need to have a separate div in the notebook for applets and not kill the applet unless it is not being used. Then Jmol would not revert to the starting over state each time a recalculation is done. I think that is another ticket for after these changes go in.

I suspected another layer of action was necessary to accomplish this or else it would have been done. My main interest was in getting it to work as intended.

3) Would reloading with a static image be less of an issue if the .png was the same quality as the Jmol image? I'm working on generating static images using Jmol on the server if a Java Virtual Machine is installed on the server. Many details like graceful fallback, probably to Tachyon, are still being worked out. Does this answer your questions or generate in suggestions for what needs work?

I think a high quality static image would be nice. It's true that if a notebook has a lot of applets then reloading that notebook can consume significant resources. Any attempt to mitigate this is desirable. FWIW, as far as I can determine, the s-o-g install of the new notebook functions exactly like the vanilla install. I have been using it here on top of Sage-4.8.

comment:10 Changed 8 years ago by gutow

  • Milestone changed from sage-5.0 to sage-5.1

comment:11 Changed 8 years ago by gutow

  • Description modified (diff)

comment:12 Changed 7 years ago by gutow

  • Description modified (diff)

This rebases everything to sage-5.0, updates Jmol,refactors the static imaging code to be more object oriented, and adds some doctesting of static image creation. Now that the flask notebook has been positively reviewed it is time to get this reviewed.

Last edited 7 years ago by gutow (previous) (diff)

comment:13 Changed 7 years ago by kcrisman

  • Cc ppurka added

Adding someone who has been using the new notebook and Jmol stuff for a while, as far as I know.

I personally won't be able to test this until the changes ahead are merged in Sage, as I'm too lazy to follow the instructions to get the Flask notebook in. But #9238 worked pretty well the last time I tried it a year ago!

comment:14 follow-up: Changed 7 years ago by kcrisman

Scarily, I managed to actually get the new notebook installed. Apparently.

Some comments:

  • I think you have to update trac_12299_plot_3D_static_img.patch. I see three different changesets to plot/plot3d/base.pyx, all of which seem to do something similar (namely, ask plot3d to make a Tachyon picture if Jmol fails). My guess is that you just exported the patches on top of each other; that's happened to me in the past too.
  • In fact, I think the same thing has happened (again!) with the sagenb patch as strogdon indicates happened before.
  • Sadly, even the __init__ method needs at least a minimal docstring and test in your new interface class. There are also some extremely minor formatting issues in that file, but clearly no need to work on that until the rest has been reviewed.

comment:15 follow-ups: Changed 7 years ago by kcrisman

My first attempt at getting this to work showed that

  • the Tachyon "placeholder" evaluates fine, though I agree with strogdon from #9238 that it's annoying to always have to make it interactive, though I understand the reasoning
  • Nothing else worked - lots of messages about restarting Jmol and so forth. At one point I even got a JmolData java applet starting outside the browser window.

Too late now to see what I did wrong, but wanted to let you know all was not flawless :( sorry.

comment:16 in reply to: ↑ 15 Changed 7 years ago by gutow

Can you add some more information as I'm not sure I understand what you are seeing?

Replying to kcrisman:

My first attempt at getting this to work showed that * the Tachyon "placeholder" evaluates fine, though I agree with strogdon from #9238 that it's annoying to always have to make it interactive, though I understand the reasoning

1) Are you running locally or from a server? 2) Is this a tachyon image or a Jmol image (Jmol looks just like what you would see in Jmol, the Tachyon image has no axis ticks/numbers)? This is important, because your problems may relate to how I am handling the case where the server does not have a JVM installed.

  • Nothing else worked - lots of messages about restarting Jmol and so forth. At one point I even got a JmolData java applet starting outside the browser window.

If you are running locally you should see the JmolData? application launch to generate the static image. So I think that answers my question about your test environment (local with a JVM available, right?). What browser are you running and did you run the notebook in secure mode? I assume you are getting the "Trying to launch JmolApplet#X?..." dialogs? This means your browser is not getting the applets from the server or is taking a very long time to load them. Any more details you have would be useful (OS, flask notebook version, etc...).

Thanks for the report.

comment:17 in reply to: ↑ 14 Changed 7 years ago by gutow

Replying to kcrisman:

Scarily, I managed to actually get the new notebook installed. Apparently.

Some comments:

  • I think you have to update trac_12299_plot_3D_static_img.patch. I see three different changesets to plot/plot3d/base.pyx, all of which seem to do something similar (namely, ask plot3d to make a Tachyon picture if Jmol fails). My guess is that you just exported the patches on top of each other; that's happened to me in the past too.
  • In fact, I think the same thing has happened (again!) with the sagenb patch as strogdon indicates happened before.

This is odd. I only see one patch in the file...I'll check it again. I also don't understand how you can get multiple patches if you only make one commit. Is something finicky about hg within sage?

  • Sadly, even the __init__ method needs at least a minimal docstring and test in your new interface class. There are also some extremely minor formatting issues in that file, but clearly no need to work on that until the rest has been reviewed.

I will work on the doctest. That's not a big deal. Thanks for pointing it out. I missed that one.

Changed 7 years ago by gutow

update to remove duplicate patches

comment:18 Changed 7 years ago by gutow

  • Description modified (diff)

comment:19 Changed 7 years ago by gutow

  • Description modified (diff)

comment:20 Changed 7 years ago by gutow

  • Description modified (diff)
  • Milestone changed from sage-5.1 to sage-5.2

comment:21 Changed 7 years ago by gutow

  • Description modified (diff)

comment:22 Changed 7 years ago by kcrisman

  • Authors changed from gutow to Jonathan Gutow
  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to needs_work
  • Work issues set to create upstream pull request

Okay, I will still have to look at what happens if "old" Jmols live in a directory, but at least it does, in fact, work.

I have to say that it is already, even after limited testing:

  • Really cool to be able to do the mesh, color, spin, everything from that advanced screen immediately - I don't think three.js will do all that right away. I remember this from testing last summer, too, but it's still nice it still works. And the extra features of the right-click menu - setting x, y, z spin separately? Nice.
  • Really annoying that even the very first Jmol on a worksheet, even a new one I create on a new worksheet (!) has the clunky "make active" thing which takes ages. We have to wait a while for 3D graphics anyway; let's not make it longer. Is there any way to make at least the first Jmol detected be a real Jmol, or is that a pipe dream?

Other minor things noticed thus far:

  • If you resize the applet using the right-click menu (say, to 200%) then the advanced menu on the right does not resize with it, which can lead to things being not very visible. Also makes it very hard to find the right-click menu.
  • Thinking out loud... should even more things from the right-click menu be on the advanced menu? That could be another ticket, but if it's really easy to do. Especially showing the axes would be REALLY nice as an option, since the axes Sage provides are (as we discussed at #9238) just plain wrong.
  • I think the right-click menu needs to be reorganized. In particular, "View" to me implies I will get to pick things to view - like axes and boxes. Now that I see what it actually does, it's really cool, but maybe "Views" would be better for that, and "Show" for some of the other options in "Style". That said, some of the things under "Style" do make sense.
  • In the right-click menu, why can't "Color" do the color of the plot itself? I was ambivalent about "Color" being a top-level one, but if it can only color the subsidiary objects, that's probably not ideal to have at the top level.
  • Sometimes I've had trouble on the right-click menu "making it over the hump" between sections; I've had to click explicitly, rather than have the submenus show up. It seems like this only happens after the first click on the menu after the right-click for the menu to appear. The subsubmenus also require explicit clicking. I have a suspicion there isn't much you can do about this.
  • So I don't remember the "high quality" button from last summer. What exactly does that do, and why wouldn't one want this to be a default? This is a very naive question, just because "high quality" sounds like it's, well, higher quality, and it doesn't seem to take any more computation time.

Okay, this is all based on pretty much just testing ONE applet. I have not tested worksheets with hundreds, or even twos, of applets. But the basic functionality is fine as before, so hopefully we can get this in for Sage 5.2 along with the new notebook - let's keep our fingers crossed. strogdon, if you're out there, this would be great to test.

Finally, I will put this as "needs work" for two reasons:

  • First, because it looks like your stuff for the sagenb has not been reported upstream and made a pull request. I know, you just learned Mercurial, and now they're making you use Git... I truly feel your pain, and am very annoyed about it. Apparently the saving grace is that in the mid-term future, it will make it possible to submit patch requests from your browser. But for now it gives me great growing pains as well, aargh.
  • Second, although I mentioned it obliquely in comment:14, I think I was too subtle - trac_12299_adv_jmol_nb.patch is also still "double-committed". This happens, if I recall correctly from when I've done it myself, when one doesn't remove the original patch foo.patch and then does hg export tip > foo.patch - apparently > must concatenate, not replace, the file, though everything I find online about bash indicates it shouldn't do this.
Last edited 7 years ago by kcrisman (previous) (diff)

comment:23 follow-up: Changed 7 years ago by kcrisman

  • Work issues changed from create upstream pull request to create upstream pull request, double patch

comment:24 in reply to: ↑ 23 ; follow-ups: Changed 7 years ago by gutow

Replying to kcrisman: OK, I see the double patch in the notebook patch...I'll fix it. I've also opened an issue on git, but I don't think that is what you meant by a pull request. Do I need to make a branch?

comment:25 in reply to: ↑ 24 Changed 7 years ago by ppurka

Replying to gutow:

Replying to kcrisman: OK, I see the double patch in the notebook patch...I'll fix it. I've also opened an issue on git, but I don't think that is what you meant by a pull request. Do I need to make a branch?

These are the steps you can follow:

  1. If you don't have a fork of the sagemath/sagenb github, then go to this page and click on "fork" on the top right. This will create a fork of this git repository inside your account.
  2. In your own machine, clone your fork using
    git clone git://github.com/<your username>/sagenb sagenb-github
    

In particular, you can follow the directions here if you don't have a git repo.

  1. Next, make the changes to the files you want (for instance you may apply your patch manually) and then commit your changes using
    git add <filenames> # In case you have introduced new files
    git commit -a -m "commit message"
    
  2. Then push your changes to your own github repo using
    git push -u origin
    
  3. Finally go to your github page and click on "Pull Request" on the top. This will create the pull request to sagemath/sagenb repository.

You can commit and push changes to the github page and the pull request will be updated automatically with your new changes. Because of this feature you can also decide to create a separate git branch and put a pull request only from that branch, so that other changes that you are working on don't get affected or included in the pull request.

If you are unclear about anything, just ask. I, kini, ohanar, are available on #sagemath too and will be glad to help. If you already know how to use git and github, then sorry for wasting your time. :)

comment:26 in reply to: ↑ 24 Changed 7 years ago by kcrisman

  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.
  • Work issues changed from create upstream pull request, double patch to double patch

OK, I see the double patch in the notebook patch...I'll fix it. I've also opened an issue on git, but I don't think that is what you meant by a pull request. Do I need to make a branch?

Well, in principle as long as you have a patch based on that sagenb, someone like kini or ppurka can do the rest :)

This is issue 54 at the sagenb github.

Changed 7 years ago by gutow

update to remove duplicate patches

comment:27 Changed 7 years ago by gutow

  • Work issues changed from double patch to pop-up menu?

comment:28 follow-up: Changed 7 years ago by strogdon

I've finally installed the flask notebook and the patches from this ticket on top of Sage-5.0. At one point things were changing too quickly for me to keep up but the situation now seems to be fairly stable. I have the following observations:

  1. The png images I see when a worksheet is first loaded are of much higher quality than what I observed with previous versions. This is nice.
  2. Occasionally, when an applet is made Interactive and the controls are toggled to "Download this view", if the download is cancelled then the Advanced controls disappear. The cell has to be re-evaluated to recover the controls. This usually happens after a first "Make Interactive" request for any one applet. Subsequent requests to Download do not seem to cause the Advanced controls to disappear.
  3. When a download is requested I get two requests to save when I cancel saving the jmol file.
  4. Is anyone able to get the grid to appear on the small red sphere for
x, y = var('x y');W = plot3d(sin(pi*((x)^2+(y)^2))/2,(x,-1,1),(y,-1,1), color='purple', opacity=0.8);S = sphere((0,0,0),size=0.3, color='red', aspect_ratio=[1,1,1]);show(W + S, figsize=4)

If no objections, I'll set myself as a reviewer also (provided I can figure out how to do that) since I've spent some time understanding and implementing items from the ticket.

comment:29 Changed 7 years ago by strogdon

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Steven Trogdon

comment:30 in reply to: ↑ 28 ; follow-up: Changed 7 years ago by gutow

Replying to strogdon:

  1. Occasionally, when an applet is made Interactive and the controls are toggled to "Download this view", if the download is cancelled then the Advanced controls disappear. The cell has to be re-evaluated to recover the controls. This usually happens after a first "Make Interactive" request for any one applet. Subsequent requests to Download do not seem to cause the Advanced controls to disappear.

This is some kind of communication issue. Probably associated with switching from unsigned to signed applets. What do you think about always using the signed applet? This will generate that warning about access to your computer the first time it is launched.

  1. When a download is requested I get two requests to save when I cancel saving the jmol file.

Yeah, that is annoying. It's something about the Java use of the save dialog. If I can catch Bob Hanson not working on something interesting, maybe we can fix that. We do know about it.

  1. Is anyone able to get the grid to appear on the small red sphere for
x, y = var('x y');W = plot3d(sin(pi*((x)^2+(y)^2))/2,(x,-1,1),(y,-1,1), color='purple', opacity=0.8);S = sphere((0,0,0),size=0.3, color='red', aspect_ratio=[1,1,1]);show(W + S, figsize=4)

I believe the sphere is a sphere object, which is not a surface defined by a bunch of vertices and triangles, thus there is no mesh to display. If Sage were to calculate it as a pmesh surface rather than use the Jmol builtin, then there would be a mesh.

comment:31 in reply to: ↑ 30 Changed 7 years ago by kcrisman

strogdon - you have totally been reviewing this, quite a bit, in fact!

  1. Occasionally, when an applet is made Interactive and the controls are toggled to "Download this view", if the download is cancelled then the Advanced controls disappear. The cell has to be re-evaluated to recover the controls. This usually happens after a first "Make Interactive" request for any one applet. Subsequent requests to Download do not seem to cause the Advanced controls to disappear.

This is some kind of communication issue. Probably associated with switching from unsigned to signed applets. What do you think about always using the signed applet? This will generate that warning about access to your computer the first time it is launched.

I really like not having to deal with that. Given that most people will not be downloading the view, the current behavior seems ok.

I believe the sphere is a sphere object, which is not a surface defined by a bunch of vertices and triangles, thus there is no mesh to display. If Sage were to calculate it as a pmesh surface rather than use the Jmol builtin, then there would be a mesh.

I don't know how Jmol does it, but you are right that we have primitives for things like spheres.

comment:32 in reply to: ↑ 15 ; follow-up: Changed 7 years ago by kcrisman

My first attempt at getting this to work showed that

  • the Tachyon "placeholder" evaluates fine, though I agree with strogdon from #9238 that it's annoying to always have to make it interactive, though I understand the reasoning

You were right, it was the Jmol placeholder. I really find that annoying.

  • Nothing else worked - lots of messages about restarting Jmol and so forth. At one point I even got a JmolData java applet starting outside the browser window.

Too late now to see what I did wrong, but wanted to let you know all was not flawless :( sorry.

I had the same problems on the same worksheet. This worksheet had only five Jmols, but took a million years to load (well, over a minute) and then I get the following.

Jmol Applet #0 is having trouble loading.  Will retry once.

Second attempt to finish launch of Jmol Applet #0 failed.  Recommend reevaluating the cell manually.

Same for all Jmols. Some didn't even make the static images. Even re-evaluating doesn't help at all - it does create the image, but as soon as I make it "live" I get the same horrible messages.

Note that in the new worksheet, I am just fine, and the little Jmol logo shows up properly in the upper left corner - not ever in this old worksheet, or only with the briefest of appearances. Notice that in old interacts everything works fine, presumably because these are indeed newly generated!


Question about implementation. How much work does Jmol have to do to generate all those static images every time that one opens a worksheet? In theory, once there is an image (png, I guess) generated, it should just stay in the cell's folder and be displayed; we certainly don't want to create a new one each time, any more than we do this for 2D plots. Though even with a largish sheet it doesn't seem to take super long, a little delay.

Have you tried using this on "old" worksheets with Jmols? This seems to be the problem I have, consistently; any Jmols from the old notebook (by which old Jmol is presumably meant) don't work.


More minor issues.

  • When I sleep an image, it doesn't come back with the advanced controls if I had them up. Is there any way to "save" that state, or is it too hard?
  • Although it's pretty clear by the content, it's a little hard to see which tab you are on in the advanced controls sometimes. Maybe the shading could be slightly different.
  • In interacts, I think there is no point to having the initial static image. I have no idea if there is a way to detect this, but presumably one wouldn't have asked for this to be evaluated if one wasn't going to use it.
  • The opening of the "JmolData" app each time is a little annoying.
  • When doing
    implicit_plot3d(x^2+y^2+z^2==16,(x,-5,5),(y,-5,5),(z,-5,5),color='green',plot_points=200)
    
    I got
    viewer handling error condition: java.lang.OutOfMemoryError: Java heap space
    
    Maybe I'm being overoptimistic; the docs don't even have any with 100 plot_points.
  • On the plus side, the thing where only four Jmols are active at a time is working fine. I notice that it deactivates the one furthest down or something, not the oldest one.

I have to say that I'm by far the most concerned about the "old worksheets" issue. Second is the "automatically every image is static" - couldn't at least the first one be "live"? - but nearly everything else could be considered an enhancement request. Let me know what stuff I've mentioned is worth discussing here.

Last edited 7 years ago by kcrisman (previous) (diff)

comment:33 Changed 7 years ago by kcrisman

  • Work issues changed from pop-up menu? to old worksheets don't work

comment:34 Changed 7 years ago by kcrisman

Update: at least sometimes re-evaluating seems to remove the problem. But it isn't consistent, needs more testing.

comment:35 in reply to: ↑ 32 ; follow-up: Changed 7 years ago by gutow

Replying to kcrisman:

My first attempt at getting this to work showed that

  • the Tachyon "placeholder" evaluates fine, though I agree with strogdon from #9238 that it's annoying to always have to make it interactive, though I understand the reasoning

You were right, it was the Jmol placeholder. I really find that annoying.

We potentially could get rid of this with significant browser detection and a complete reworking of how Jmol is called for when a cell is recalculated. The problem is related to the fact that the applet is completely reloaded each time a cell is recalculated, rather than just passing it new data. This means it is very easy to get out of sync with what the server is sending to the applet. Many of the problems with the old Jmol relate to this. One of the problems I consistently have using the old format where Jmol loads live in interacts is that the interact starts calling for a new applet before the previous one is finished loading. I have had some discussions about putting the applet in a special div that does not get zapped each time a recalculation is done. That's probably the correct thing to do, but not high on people's list when the notebook was undergoing major revisions. I would prefer to put this off as an additional enhancement.

  • Nothing else worked - lots of messages about restarting Jmol and so forth. At one point I even got a JmolData java applet starting outside the browser window.

As your comments below suggest this appears to be a problem with old worksheets that have not been reevaluated. We might be able to put something into the notebook code that puts up a message suggesting the worksheet be re-evaluated if it does not have the data necessary for static images. I tried to put a message in the "alt" field for the images, but most browser now seem to ignore that. I think I can put a check into the cell code that would deal with this. I even have some ideas for generating images without reevaluation...I'll look into this.

Question about implementation. How much work does Jmol have to do to generate all those static images every time that one opens a worksheet?

Jmol only generates the image when a cell is evaluated. When you open a worksheet that was previously evaluated using these enhancements the images are already there.

More minor issues.

  • When I sleep an image, it doesn't come back with the advanced controls if I had them up. Is there any way to "save" that state, or is it too hard?

Hadn't thought about this. I will look into it.

  • Although it's pretty clear by the content, it's a little hard to see which tab you are on in the advanced controls sometimes. Maybe the shading could be slightly different.

I'm using Sage default CSS. I suppose we could define something custom.

  • In interacts, I think there is no point to having the initial static image. I have no idea if there is a way to detect this, but presumably one wouldn't have asked for this to be evaluated if one wasn't going to use it.

As I explained above, only simple interacts where the user does not call for lots of recalculations work reliably with the present model of loading Jmol live. Things with sliders are really bad.

  • The opening of the "JmolData" app each time is a little annoying.
  • When doing
    implicit_plot3d(x^2+y^2+z^2==16,(x,-5,5),(y,-5,5),(z,-5,5),color='green',plot_points=200)
    
    I got
    viewer handling error condition: java.lang.OutOfMemoryError: Java heap space
    
    Maybe I'm being overoptimistic; the docs don't even have any with 100 plot_points.

The application can certainly handle more. I'll see if I can change the memory allocation to allow for larger images...That is a little odd as we use Jmol in web pages to look at things with many more points, atoms and surfaces...

  • On the plus side, the thing where only four Jmols are active at a time is working fine. I notice that it deactivates the one furthest down or something, not the oldest one.

Actually it picks the one farthest from the cell you most recently evaluated that includes a Jmol. It can be above or below the cell you are working on.


I have to say that I'm by far the most concerned about the "old worksheets" issue. Second is the "automatically every image is static" - couldn't at least the first one be "live"? - but nearly everything else could be considered an enhancement request. Let me know what stuff I've mentioned is worth discussing here.

I agree that this should be a priority. I think there are solutions. What behavior would be best: 1) A pop-up suggestion to reevaluate old worksheets to update 3-D plots; 2) Attempt to update plots without reevaluation; 3) Automatically reevaluate worksheets with 3-D plots in them?

comment:36 in reply to: ↑ 35 ; follow-up: Changed 7 years ago by kcrisman

We potentially could get rid of this with significant browser detection and a complete reworking of how Jmol is called for when a cell is recalculated. The problem is related to the fact that the applet is completely reloaded each time a cell is recalculated, rather than just passing it new data. This means it is very easy to get out of sync with what the server is sending to the applet. Many of the problems with the old Jmol relate to this.

Right, and I assume that passing it new info would be a pretty sizable change in how we use it.

I would prefer to put this off as an additional enhancement.

Now that I understand the technical issue better, I guess I would agree in some sense, but I figure it's buyer beware on the @interact cells. Having to do an extra click every time would get really old really fast with them. Maybe we could ask on sage-devel about the tradeoff, though perhaps (as usual with polls on sage-devel) we wouldn't get many takers...

As your comments below suggest this appears to be a problem with old worksheets that have not been reevaluated. We might be able to put something into the notebook code that puts up a message suggesting the worksheet be re-evaluated if it does not have the data necessary for static images. I tried to put a message in the "alt" field for the images, but most browser now seem to ignore that. I think I can put a check into the cell code that would deal with this. I even have some ideas for generating images without reevaluation...I'll look into this.

We definitely need to fix this. Again, I even had trouble with some worksheets even after re-evaluation.

It should be pretty easy for you to test this. Just open an old Sage, make a new worksheet with Jmols, close it, stop Sage, restart your new Jmol Sage, and see what happens. I figure you have more diagnostic tools than I will.

Jmol only generates the image when a cell is evaluated. When you open a worksheet that was previously evaluated using these enhancements the images are already there.

Great.

The application can certainly handle more. I'll see if I can change the memory allocation to allow for larger images...That is a little odd as we use Jmol in web pages to look at things with many more points, atoms and surfaces...

No, it's 100 points per axis, so one million points are evaluated and the presumably however many of them are on the sphere must be plotted. I think that's why we don't even have it in the docs, because it would take a long time. But it does seem to work in the old Jmol in Sage (just tried it in vanilla 5.0).

I agree that this should be a priority. I think there are solutions. What behavior would be best: 1) A pop-up suggestion to reevaluate old worksheets to update 3-D plots; 2) Attempt to update plots without reevaluation; 3) Automatically reevaluate worksheets with 3-D plots in them?

Definitely not 3. 1 also seems less than desirable unless it was only when there had been a Jmol before, and one could reliably detect it was from an earlier version of Jmol. How would 2 work?


Some dumb generic review questions.

  • So far I've not been able to use this except on Safari on OS X because of some default I can't get around and logins. What have you tested this on? In the past you had done some pretty extensive testing. I figure we should at least test on Safari, FF, Chrome on Mac, FF and Chrome on Linux, IE and FF on Windows. I don't know how easy that will be for me unless you set up a remote server again, which I figure would be quite a pain.
  • What problems should one be looking for with secure=True? It sounds like you are thinking that there could be different issues in that case, maybe because of the applets being applets?

comment:37 Changed 7 years ago by gutow

  • Description modified (diff)

comment:38 in reply to: ↑ 36 ; follow-up: Changed 7 years ago by gutow

Replying to kcrisman:

Some dumb generic review questions.

  • So far I've not been able to use this except on Safari on OS X because of some default I can't get around and logins. What have you tested this on? In the past you had done some pretty extensive testing. I figure we should at least test on Safari, FF, Chrome on Mac, FF and Chrome on Linux, IE and FF on Windows. I don't know how easy that will be for me unless you set up a remote server again, which I figure would be quite a pain.

I will keep a server up as best I can for the time being. Go to TestServer and create yourself an account. I will try to keep it up-to-date and running, but it may be down when I am coding. Linux and MacOS, I can test pretty well, but I really need feedback on Windows as I do not have a recent version.

  • What problems should one be looking for with secure=True? It sounds like you are thinking that there could be different issues in that case, maybe because of the applets being applets?

The major issue I am worried about is reliability of the applets when an unsigned applet is used in a secure environment. Browsers and Java are getting much more finicky about mixed modes. I've seen some cases where the applet will not launch under https if it is not signed. I need to know whether this is enough of a problem to spend some time on.

comment:39 in reply to: ↑ 38 ; follow-up: Changed 7 years ago by kcrisman

  • So far I've not been able to use this except on Safari on OS X because of some default I can't get around and logins. What have you tested this on? In the past you had done some pretty extensive testing. I figure we should at least test on Safari, FF, Chrome on Mac, FF and Chrome on Linux, IE and FF on Windows. I don't know how easy that will be for me unless you set up a remote server again, which I figure would be quite a pain.

I will keep a server up as best I can for the time being. Go to TestServer and create yourself an account. I will try to keep it up-to-date and running, but it may be down when I am coding. Linux and MacOS, I can test pretty well, but I really need feedback on Windows as I do not have a recent version.

  • What problems should one be looking for with secure=True? It sounds like you are thinking that there could be different issues in that case, maybe because of the applets being applets?

The major issue I am worried about is reliability of the applets when an unsigned applet is used in a secure environment. Browsers and Java are getting much more finicky about mixed modes. I've seen some cases where the applet will not launch under https if it is not signed. I need to know whether this is enough of a problem to spend some time on.

Is the test server you link to with secure=True or False? I guess since it's http, not https, it's False?

comment:40 in reply to: ↑ 39 Changed 7 years ago by gutow

Replying to kcrisman:

Is the test server you link to with secure=True or False? I guess since it's http, not https, it's False?

Correct. Let's get other issues dealt with before the signed issue.

comment:41 Changed 7 years ago by strogdon

So I have a situation here that I hadn't notice previously. I have 12 applets in a worksheet. I'm not sure if the number is important but probably at least 4 are needed. If I start the notebook and load the worksheet things seem normal; I can "Make Interactive" applets throughout the worksheet with 4 interactive ones. So applets are put to sleep as new ones are made interactive. Now if I "Toggle Advanced Controls" on one of the applets, proceed to "Download this view" and cancel the request; I'm left with 3 interactive applets. As I toggle the advanced controls now to make applets interactive there are never more than 3 interactive applets. At some point in the clicking process to make applets interactive I get a pop-up with

Could not find jmolApplet#

where # is some number. After the pop-up is closed, things get frozen. I'm unable to make applets interactive unless I put to sleep an already interactive applet. I've tried this, at leas 4 times with the same result. Now I'm doing this over nfs. I wouldn't think that nfs would cause this. What seems to recover things is the sequence; Actions -> Delete All Output and then Actions -> Evaluate All. I can then get 4 interactive applets again, all of which are now signed. If repeat the procedure to "Download this view" and cancel the request with these signed applets the worksheet doesn't freeze.

comment:42 follow-up: Changed 7 years ago by strogdon

In trying to isolate exactly what triggers the above I found that if I don't try to recover the Advanced Controls on the applet that was used to download the view as a jmol file (Download this view) then the worksheet doesn't seem to freeze as I make other applets interactive. And eventually this applet that was used to download the view as a jmol will be put in a sleep state that can then be made interactive. So, it seems that if an applet is unsigned and one wishes to save the view in a jmol file, to avoid freezes the best thing to do is to not re-evaluate its cell.

comment:43 in reply to: ↑ 42 Changed 7 years ago by gutow

Thanks. This gives me a sequence I can follow to see what is happening. I believe there is a collision between signed and unsigned applets. I may have to convert all open applets to signed...This may take a while to figure out, but I think it is important. Replying to strogdon:

In trying to isolate exactly what triggers the above I found that if I don't try to recover the Advanced Controls on the applet that was used to download the view as a jmol file (Download this view) then the worksheet doesn't seem to freeze as I make other applets interactive. And eventually this applet that was used to download the view as a jmol will be put in a sleep state that can then be made interactive. So, it seems that if an applet is unsigned and one wishes to save the view in a jmol file, to avoid freezes the best thing to do is to not re-evaluate its cell.

comment:44 Changed 7 years ago by gutow

  • Work issues changed from old worksheets don't work to old worksheets don't work, signed and unsigned applet collision

comment:45 Changed 7 years ago by kcrisman

Here's an interesting thing I wasn't expecting.

More than 9 Jmols have been launched on the worksheet since it was last openned. Unable to get a static image from Jmol#0 You might want to try Chrome as this problem does not exist with Chrome on MacOS.

And indeed, a few have a little message to that effect, and no static image. Looks like something added on the previous ticket, though I'm using Safari, not FF, on Mac.

So one could sort of jam up a worksheet. I was able to get all the plots to vanish by playing a little game where I chased them around, and then got the advanced menu to try to generate a static image, which would (intentionally, I guess) fail. What is the story behind this? Can Jmol not detect that there are no longer any live images, and hence try again to generate a static one, or would this be hard to work around?

I do note that quitting and reopening the page gives normal behavior. But just restarting inside the worksheet doesn't.

I tried to recreate strogdon's signed applet issue. I couldn't get exactly that, but I did note that once the signed applet has been requested (not downloaded, as in his), there doesn't seem to be any way to go back to the unsigned applet - even reevaluating the cell doesn't do it.


I also tried to see if I could get away from the problems with old Jmols, but try as I might it wouldn't work. Re-evaluating will sometimes generate a png for the next time one opens the page, but I can't get it to consistently "just work". Now sometimes even new cells in this old worksheet give the same error message. It's puzzling and frustrating.

comment:46 Changed 7 years ago by kcrisman

I am trying with secure=True. Other than a few extra messages, I don't notice any difference. The only thing is that once I try to download an applet, then all the applets I wake up after that point have the big red Jmol_S thing that strogdon noted above. But otherwise there doesn't seem to be a problem on Safari 5, at least.

I like this enough, even with the work issues, that I'm going to ask on sage-devel if anyone cares about having to wake things up by default. If there is enough outcry, this would have to be dealt with somehow.

comment:47 Changed 7 years ago by gutow

Replying to kcrisman:

Here's an interesting thing I wasn't expecting. More than 9 Jmols have been launched on the worksheet since it was last openned. Unable to get a static image from Jmol#0 You might want to try Chrome as this problem does not exist with Chrome on MacOS. And indeed, a few have a little message to that effect, and no static image. Looks like something added on the previous ticket, though I'm using Safari, not FF, on Mac. 

This is a limitation of Safari memory allocation, that I haven't double checked on in a while. Your problems indicate that it has not changed. I will check, but I do not think I can fix it. It appears to be a memory leak in MacOS Safari. This is why you get the suggestion to switch to Chrome, which does not have this problem. The symptom I saw was If you open more than a certain number of java applets on a page in sequence, even if they are not left open, things get messed up.

I also tried to see if I could get away from the problems with old Jmols, but try as I might it wouldn't work. Re-evaluating will sometimes generate a png for the next time one opens the page, but I can't get it to consistently "just work". Now sometimes even new cells in this old worksheet give the same error message. It's puzzling and frustrating.

I have found the problem. There is a change in the way the flask notebook provides information about the path to the cell data. This effects the command script that launches Jmol. I hope to have time tonight to write a fix. I haven't tested this yet, but I think that you should see the same problem with old worksheets in the flask notebook without the enhancements from this ticket.

Changed 7 years ago by gutow

fix Jmol errors on loading pre-flask worksheets

comment:48 Changed 7 years ago by gutow

  • Description modified (diff)
  • Work issues changed from old worksheets don't work, signed and unsigned applet collision to signed and unsigned applet collision

In the old worksheet patch I chose to generate static images if they do not exist. An alternative is just to fix the path so Jmol loads and print a message suggesting the cell be re-executed to generate a static image. Even without the image the interactive Jmol should now load properly. The advantage of the alternative is that no calls to the sage server would be necessary. The image generation depends on Sage+Jmol. It is not done by the notebook.

Last edited 7 years ago by gutow (previous) (diff)

comment:49 follow-up: Changed 7 years ago by kcrisman

  • Work issues changed from signed and unsigned applet collision to signed and unsigned applet collision, old worksheets may not work all the time

No luck. I get the same error messages, re-evaluating doesn't help. I also get a new message, quite a while after the original messages recommending re-evaluating etc.

could not find applet jmolApplet0

I suppose it could just be this one worksheet, but at any rate this definitely needs more testing. I should point out that your new patch seems to only affect the generation of the static images, from how I read it, and that part is fixed, they are fine. Maybe there also is trouble finding the live applets in a similar way?

Last edited 7 years ago by kcrisman (previous) (diff)

comment:50 in reply to: ↑ 49 ; follow-up: Changed 7 years ago by gutow

Replying to kcrisman: e-mail me the worksheet please, (gutow@…). This is odd.

comment:51 in reply to: ↑ 50 Changed 7 years ago by kcrisman

e-mail me the worksheet please, (gutow@…). This is odd.

I uploaded it to the test server first. I tried to share it with you, but maybe you didn't have an account on it. I'll email it as well. It doesn't work right there, either.

By the way, I got

Jmol Applet #0 is having trouble loading.  Will retry once.

on the test server for a new worksheet, but then it eventually did come up fine and so forth.

Remind me why we aren't loading the "High Quality" view right away? I'm sure there's a good reason, but it just looks so much nicer that I'm surprised we don't just use it by default.

comment:52 Changed 7 years ago by kcrisman

Also, sometimes timing seems to be off. On the test server, I made a cell on a page with an already-functioning Jmol image, and got

script ERROR: script ERROR: io error reading
/home/sagetest/.sage/sage_notebook.sagenb/home/__store__/9/9e/9eb/9eba/k\
crisman/0/cells/3/sage0-size500-747143461.jmol.zip|SCRIPT:
java.io.FileNotFoundException:
/home/sagetest/.sage/sage_notebook.sagenb/home/__store__/9/9e/9eb/9eba/k\
crisman/0/cells/3/sage0-size500-747143461.jmol.zip (No such file or
directory)

But then when I re-evaluated it, all was well. Maybe this is unavoidable in any case with a remote server on occasion.

comment:53 Changed 7 years ago by kcrisman

Different note. It's possible to mess things up (empty Jmol areas, unable to load messages, dying areas) by clicking to wake up applets really quickly, six or seven in a row. I don't think that's really a regression, because before they would have just been awake and taking up memory, but at any rate I wanted to let you know. This is on FF on Mac, the worst-case scenario (though it does work despite the nasty-sounding warning message I get!).

On Chrome on Mac, no problems.

comment:54 follow-up: Changed 7 years ago by kcrisman

Trying a (virtual) Windows XP right now.

  • IE 8 - no dice. I do get the "If no image appears ..." message about the 3-D viewer being updated, but re-evaluating does not help.
    • Jmol does work on the non-flask notebook (I tried a server I have access to)
    • Jmol does work on sagenb.org, which has the flask notebook but not this particular update
    • I updated Java, but that didn't seem to help.
  • FF3.5 (the newest I have on that machine) it seems to work fine.
  • Chrome works fine. On both of these it didn't like my old Java but allowed me to click through.

Weirdly, I do not have trouble with my suspect worksheet in certain cases! All on the test server:

  • Using Chrome on XP
  • Using FF on XP
  • Using FF on Mac

But it still doesn't work with Safari on Mac (just the old worksheet, and just that one).

comment:55 in reply to: ↑ 54 Changed 7 years ago by gutow

Replying to kcrisman:

Trying a (virtual) Windows XP right now.

  • IE 8 - no dice. I do get the "If no image appears ..." message about the 3-D viewer being updated, but re-evaluating does not help.

Since this seems to be related to issues of IE8 handling jquery functions differently depending on where they are on the page, I don't think I can fix this easily.

Did you at least get the warning that things were not likely to work? It should have read something like: "Many of the advanced 3-D viewing functions DO NOT work in Internet Explorer..." Or are things failing before then?

comment:56 follow-up: Changed 7 years ago by kcrisman

Regarding IE 8 on XP, as I said I get the "If no image appears" message and something about re-evaluating. I did not get the message you report.

Weirdly, I do not have trouble with my suspect worksheet in certain cases! All on the test server:

  • Using Chrome on XP
  • Using FF on XP
  • Using FF on Mac

But it still doesn't work with Safari on Mac (just the old worksheet, and just that one).

Update: gutow discovered this was due to a weird Geogebra applet embedding attempt left in that particular worksheet that for some reason was causing the trouble.

Also, based on the sage-devel discussion, sounds like people are ok with not having immediate interaction, and obviously Jonathan is worry about overlapping requests to Jmol. I'm still not sure this is advisable, but be it on their heads.

I'd still ask for some reorganization of the menus but that is probably for another ticket, given the current state of the applet in 5.0 being much more difficult to follow than this new one, which only needs tweaking.

At least some people are reporting success on FF on Linux. Apparently some Opera has trouble there. But maybe Linux is okay, then, at least many of them.


What remains to look at:

  • Someone to do at least brief testing on Windows 7. I will eventually be able to do that, but I'm not sure when yet.
  • The signed/unsigned issue.
  • The secure/insecure notebook issue, if it needs more testing.
  • Someone has to create an upstream pull thingie - the ticket is already open on github. I think that it would be easiest for kini to do this :) but maybe we should wait until it otherwise has positive review.
  • Anything else? This is looking good, it would be great to get it in 5.2 along with the new notebook.

comment:57 in reply to: ↑ 56 Changed 7 years ago by gutow

Replying to kcrisman:


What remains to look at:

  • Someone to do at least brief testing on Windows 7. I will eventually be able to do that, but I'm not sure when yet.

I may be able to do that at work tomorrow...

  • The signed/unsigned issue.

I cannot reproduce the problem...so I think this will have to wait until we get more info.

  • The secure/insecure notebook issue, if it needs more testing.

I think this is resolved. I cannot reproduce the hangs I sometimes saw. There is no question that loading an unsigned applet causes the initial load to slow down, but after that there seems to be no trouble.

  • Someone has to create an upstream pull thingie - the ticket is already open on github. I think that it would be easiest for kini to do this :) but maybe we should wait until it otherwise has positive review.

I was going to try, but see a problem. This ticket requires changes to both the notebook and Sage. The pull would just be for notebook, right?

  • Anything else? This is looking good, it would be great to get it in 5.2 along with the new notebook.

I'm going to clear the two work issues and replace with "decide default display quality". See this google thread 3D plotter.

comment:58 Changed 7 years ago by gutow

  • Work issues changed from signed and unsigned applet collision, old worksheets may not work all the time to decide default display quality

comment:59 Changed 7 years ago by kcrisman

Followups now that I've had time to think.

  • Somehow the 'needs work' was not fixed, sorry.
  • Responding to

    Did you at least get the warning that things were not likely to work? It should have read something like: "Many of the advanced 3-D viewing functions DO NOT work in Internet Explorer..." Or are things failing before then?

    You're right, it's failing before that.
  • We do need the final doctests - can be very skimpy.
  • Jonathan, I'm wondering whether it's worth creating a new error class for this one specific error of not having the JVM. I mean, your string is so informative, you could just raise a very generic error with that message. Also, it would eliminate two needed doctests :)
  • As for the upstream pull, you would only create one for the notebook, that's right, and that could be merged upstream at the same time. I've asked about the process for that there, in case there is anything weird.

This is looking really close to ready, though.

comment:60 Changed 7 years ago by gutow

  • Work issues changed from decide default display quality to decide default display quality, doctests,switch to generic error

comment:61 follow-up: Changed 7 years ago by strogdon

I've been able to reproduce the "signed/unsigned" applet issue under comment:41 and comment:42 on ubuntu 12.04. I don't ususally use ubuntu but had access to an installation. The behavior was observed with FF and chromium. I know there are quite a number of Sage users that install from ubuntu. I was not able to get the "Download this view" to a jmol to work at all unless I manually installed oracle-java-jre. Ubuntu only provides the icedtea-java-jre. Without the oracle jre, when I clicked the "Download this view" there appeared in the applet pane

File reader was not found:org.jmol.adapter.readers.molxyz.XyzReader

and there were similar complaints in the terminal window from which Sage was started. After this happens no applet can be made interactive. A grep of the sagenb tree doesn't reveals that XyzReader? isn't called from Sage so I'm guessing it's embedded somewhere in the icedtea jre. As I mentioned the only work-around for me was to manually install oracle. Here is, I think a minimal sequence of steps to reproduce things:

  1. load a worksheet with at least 5 applets
  2. make the first applet interactive
  3. click "Toggle Advanced Controls"
  4. click "Download this view" and cancel all requests (the applet will change to "signed")
  5. re-evaluate the cell to recover the advanced controls
  6. make the applet interactive again
  7. click through the remaining applets to make them interactive

I'm able to make 2 additional applets interactive, but not the 3rd and subsequent applets. An applet can only be made interactive after I put to sleep an already interactive applet. Now I used the applet spawed by that code given in comment:28

comment:62 in reply to: ↑ 61 Changed 7 years ago by gutow

Replying to strogdon:

Here is, I think a minimal sequence of steps to reproduce things:

  1. load a worksheet with at least 5 applets
  2. make the first applet interactive
  3. click "Toggle Advanced Controls"
  4. click "Download this view" and cancel all requests (the applet will change to "signed")
  5. re-evaluate the cell to recover the advanced controls
  6. make the applet interactive again
  7. click through the remaining applets to make them interactive

Thanks. I'll try again. So far I haven't been able to reproduce this on MacOS or Linux. If I can reproduce it I'll have a chance of fixing it.

As to the IcedTea?, I think we just have to wait for them to finish fixing things. Some people have reported that the applet mostly works with IcedTea? (see 3D plotting discussion).

comment:63 Changed 7 years ago by kcrisman

I've submitted a new pull request with this same code at this Github pull request, because I couldn't figure out how to attach code to the old one. Apparently it introduced a few whitespaces, but that's really not my concern. Hopefully the patches won't need updating, or I'm in trouble; this took hours for me to figure out how to do from scratch.

Last edited 7 years ago by kcrisman (previous) (diff)

comment:64 follow-ups: Changed 7 years ago by strogdon

Another scenario that leads me to believe what I'm seeing is not a "signed/unsigned" applet issue:

  1. load a worksheet with 5 applets
  2. make the applets interactive buy starting from the top and working to the bottom of the worksheet - the applets will be unsigned.
  3. the first applet should now be sleeping, re-evaluate the second applet's cell (this applet should not be sleeping) and make the applet interactive. Here is where I get the problem. I get a pop-up and nothing more can be made interactive.

This can be reproduced if the applets are singed as well. Yet another approach that may shed some light provided someone can get it to behave as here:

  1. load a worksheet with 5 applets
  2. make the applets interactive, from top to bottom, in the order 1 - 5 - 2 - 4 - 3
  3. the first applet should be sleeping but the second one should be interactive
  4. re-evaluate the second applet's cell and make the applet interactive
  5. Here I only get 3 interactive applets, regardless of how many are made interactive.

This is a linux install from a gentoo-prefix where the host OS is debian. The browser is FF.

comment:65 in reply to: ↑ 64 Changed 7 years ago by kcrisman

  1. load a worksheet with 5 applets
  2. make the applets interactive buy starting from the top and working to the bottom of the worksheet - the applets will be unsigned.
  3. the first applet should now be sleeping, re-evaluate the second applet's cell (this applet should not be sleeping) and make the applet interactive. Here is where I get the problem. I get a pop-up and nothing more can be made interactive.

I can reproduce this on Mac OS with Safari.

could not find applet jmolApplet1

After that once in a while I was able to get one to show up interactive, but most just have

Loading Jmol 3-D viewer...

where the applet would be, and then of course eventually, as it's Safari,

More than 9 Jmols have been launched on the worksheet since it was last openned. Unable to get a static image from Jmol#1 You might want to try Chrome as this problem does not exist with Chrome on MacOS.

which incidentally has a typo (extra "n").

comment:66 in reply to: ↑ 64 ; follow-up: Changed 7 years ago by gutow

  • Work issues changed from decide default display quality, doctests,switch to generic error to applet ID when cells re-evaluated,decide default display quality, doctests,switch to generic error

Replying to strogdon:

Another scenario that leads me to believe what I'm seeing is not a "signed/unsigned" applet issue:

  1. load a worksheet with 5 applets
  2. make the applets interactive buy starting from the top and working to the bottom of the worksheet - the applets will be unsigned.
  3. the first applet should now be sleeping, re-evaluate the second applet's cell (this applet should not be sleeping) and make the applet interactive. Here is where I get the problem. I get a pop-up and nothing more can be made interactive.

Thank you now I've got it...This may take a bit to fix. It appears to be a discrepancy between the server and javascript applet numbers. There are many places where the update could be getting messed up. At least I know what to look for now. And you are correct, this should have nothing to do with whether the applets are signed or not.

comment:67 follow-up: Changed 7 years ago by kini

I've transferred the sagenb-related patches to a github pull request. Feel free to write more patches, and I'll put them there too, if you like. (Please don't overwrite old patches, that makes it more complicated.) Or I can put any other branch you like onto the pull request - just ask.

Please CC me in the future if you want me to transfer patches to github. If you're interested, here's what I did:

$ cd ~/src/sagenb # go to my sagenb repo
$ git checkout master # get on master
$ git pull # my master is set up to track sagemath/master, i.e. the latest upstream
$ git checkout -b trac_12299 # create a new branch called trac_12299
$ # download the patches to the current directory
$ git am --whitespace=fix trac_12299_adv_jmol_nb.patch
Applying: Trac 12299: add interactive Jmol controls in notebook
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:15: trailing whitespace.
    for particular browser/OS combinations.  Problem systems FF/MacOS (intermittent), 
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:18: trailing whitespace.
	
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:30: trailing whitespace.
Jmol.js 
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:37: trailing whitespace.
        the evaluate_cell() call and others that delete cell output must be modified 
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:41: trailing whitespace.
        where a 
warning: squelched 37 whitespace errors
warning: 42 lines applied after fixing whitespace errors.
$ git am --whitespace=fix trac_12299_old_wksht.patch
Applying: trac 12299: fix jmol in old worksheets
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:41: trailing whitespace.
        
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:44: trailing whitespace.
            # Note: this is problematic in the notebook as it uses tools from Sage to 
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:51: trailing whitespace.
            f.write('set defaultdirectory "%s"\n' %path) 
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:68: trailing whitespace.
                #print script 
/home/fs-boone/src/sagenb/.git/rebase-apply/patch:72: trailing whitespace.
            
warning: 5 lines applied after fixing whitespace errors.
$ git log -2 --stat
commit 32be06d65240aac183b391ef24e8149b52eabaea (HEAD, refs/remotes/origin/trac_12299, refs/heads/trac_12299)
Author: Jonathan Gutow <gutow@uwosh.edu>
Date:   Mon Jun 11 20:44:10 2012 -0500

    trac 12299: fix jmol in old worksheets

 sagenb/notebook/cell.py |   46 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

commit 1971a68138c14e7057e468802a598b05760bb733
Author: Jonathan Gutow <gutow@uwosh.edu>
Date:   Sun May 27 08:48:59 2012 -0500

    Trac 12299: add interactive Jmol controls in notebook

 sagenb/data/sage/js/jmol_lib.js     | 1233 ++++++++++++++++++++++++++++++++++-
 sagenb/data/sage/js/notebook_lib.js |    7 +
 sagenb/notebook/cell.py             |    7 +-
 3 files changed, 1223 insertions(+), 24 deletions(-)

The last command was just for me to check that the author/date/etc. looked correct, as well as the number of lines added/removed and number of files changed. There was a minor discrepancy in the larger patch, but it looks like it was just an instance of trailing whitespace being added which disappeared when git fixed the whitespace changes.

comment:68 in reply to: ↑ 67 Changed 7 years ago by kcrisman

> $ cd ~/src/sagenb # go to my sagenb repo
> $ git checkout master # get on master
> $ git pull # my master is set up to track sagemath/master, i.e. the latest upstream
> $ git checkout -b trac_12299 # create a new branch called trac_12299
> $ # download the patches to the current directory
> $ git am --whitespace=fix trac_12299_adv_jmol_nb.patch
> $ git am --whitespace=fix trac_12299_old_wksht.patch

This is some of what I was talking about in other locations, except now how does one use this in an actual "working" Sage install? (Before pull 63 on github sagenb, I mean.) From other discussion it looks like plopping this new sagenb in a Sage that already has #11080 and friends applied would suffice?

comment:69 follow-up: Changed 7 years ago by kini

Yup. In the instructions on #11080, replace the step "install the sagenb spkg" with "clone the sagenb repo into devel/sagenb (or symlink devel/sagenb to a clone of the sagenb repo), and then run sage -python setup.py develop inside devel/sagenb". You need to run sage -python setup.py develop only once (unlike sage -b for the sage library) - it is the step which pulls in all the dependencies and installs them into your $SAGE_LOCAL.

comment:70 in reply to: ↑ 69 Changed 7 years ago by kcrisman

and then run sage -python setup.py develop inside devel/sagenb". You need to run sage -python setup.py develop only once (unlike sage -b for the sage library) - it is the step which pulls in all the dependencies and installs them into your $SAGE_LOCAL.

Ah, this is the part I was probably missing.

comment:71 in reply to: ↑ 66 ; follow-up: Changed 7 years ago by gutow

Replying to gutow:

Replying to strogdon:

Another scenario that leads me to believe what I'm seeing is not a "signed/unsigned" applet issue:

Thank you now I've got it...This may take a bit to fix. It appears to be a discrepancy between the server and javascript applet numbers. There are many places where the update could be getting messed up. At least I know what to look for now. And you are correct, this should have nothing to do with whether the applets are signed or not.

I think I've fixed it. I found quite a few places I missed where the server could get out of sync with the browser. Please try my test server TestServer before I make a patch. Thanks.

comment:72 in reply to: ↑ 71 Changed 7 years ago by strogdon

Replying to gutow:

I think I've fixed it. I found quite a few places I missed where the server could get out of sync with the browser. Please try my test server TestServer before I make a patch. Thanks.

With your changes I'm unable to get a worksheet to freeze or act oddly. This looks good to me. And the original issue, that spawned all this, in attempting to download a jmol is no longer an issue.

comment:73 follow-up: Changed 7 years ago by kini

I went to the test server, created an account, plotted x^2+y^3 over (-1,1)×(-1,1), clicked "make interactive", and firefox immediately crashed. Does that count as "acting oddly"? :)

I doubt this is caused by jmol in particular or anything in this code. FWIW it's running the Sun Java 6 plugin on Debian wheezy amd64.

comment:74 in reply to: ↑ 73 ; follow-up: Changed 7 years ago by kcrisman

With your changes I'm unable to get a worksheet to freeze or act oddly.

Great!

I went to the test server, created an account, plotted x^2+y^3 over (-1,1)×(-1,1), clicked "make interactive", and firefox immediately crashed. Does that count as "acting oddly"? :)

Surprisingly, it doesn't! I think acting oddly is different from acting badly :)


I've been messing with the "high quality" issue. So, it does look enough better that it's hard not to want it, especially since memory should not be an issue at this point. I do see the speckles with the mesh on. However, they only appear with the mesh, and only with the black mesh - I can't get it with any other mesh, not even the darker grays.

Maybe we could keep the high-quality if the mesh defaults to one of those colors? Or is there an "xor" we could set, not allowed to use high quality and mesh at the same time if the color is so and so? That seems to be a little limiting, though.

The black mesh speckles have their value too - we could hypnotize people at the Joint Meetings booths...

comment:75 Changed 7 years ago by kcrisman

  • Work issues changed from applet ID when cells re-evaluated,decide default display quality, doctests,switch to generic error to decide default display quality, doctests,switch to generic error

comment:76 Changed 7 years ago by kini

I tried it again after restarting firefox - this time it seemed to work. Still takes several seconds to load the first interactive plot (which I guess is inevitable since it's a Java applet), but loading subsequent plots only takes one or two seconds each.

comment:77 follow-up: Changed 7 years ago by kini

I did notice some yellow warning icons flickering about when I opened the context menu. I'm not sure what that's about.

comment:78 in reply to: ↑ 77 Changed 7 years ago by gutow

Replying to kini:

I did notice some yellow warning icons flickering about when I opened the context menu. I'm not sure what that's about.

Maybe the way your software is reacting to all the warnings applets now flash when they make separate windows. This also depends on your Java and browser security settings. If you choose the download option, which causes you to switch to the signed applet, you should get a dialog which actually requires you to make a decision.

comment:79 Changed 7 years ago by gutow

  • Cc kini added

comment:80 follow-up: Changed 7 years ago by ppurka

There seems to be some problem with the help window at least in firefox on linux. The help window that can be popped up from within jmol actually has mouse scroll disabled and also lacks a scroll bar. But pg up/down works and shows that there is more text present once scrolled down.

I had a look at the spkg and I think inserting a css rule like this in the patches/appletweb/JmolHelp.html might do the trick. Unfortunately, I can't seem to figure out how to do live page edits in firefox, so I can't test this right away.

html {
     overflow: -moz-scrollbars-vertical;
     overflow-y: scroll;
}

comment:81 Changed 7 years ago by ppurka

And I concur with kini. I always see yellow warning signs on right click menu (and the right click menu is slow too). But that is also with the current jmol in sage-5 (in opera/firefox), so I didn't think much about it. I almost never need to use the right click with jmol anyway.

comment:82 in reply to: ↑ 74 ; follow-up: Changed 7 years ago by gutow

Replying to kcrisman:

I've been messing with the "high quality" issue.

Maybe we could keep the high-quality if the mesh defaults to one of those colors?

This is a great idea, which works! I suggest we use the darkest of the blues as that is very close to black. If there are no objections I will change the default.

comment:83 in reply to: ↑ 82 Changed 7 years ago by ppurka

Replying to gutow:

This is a great idea, which works! I suggest we use the darkest of the blues as that is very close to black. If there are no objections I will change the default.

Anything to get pretty pictures!!

comment:84 in reply to: ↑ 80 Changed 7 years ago by gutow

Replying to ppurka:

There seems to be some problem with the help window at least in firefox on linux.

Try it now. I think I've forced it to have a scrollbar (it does on FF/linux for me).

Replying to ppurka:

Anything to get pretty pictures!!

I've made the change see if you like it.

comment:85 Changed 7 years ago by ppurka

Thanks. Scroll works now, and pictures look good :)

comment:86 Changed 7 years ago by gutow

  • Work issues changed from decide default display quality, doctests,switch to generic error to doctests,switch to generic error

Changed 7 years ago by gutow

Fix server<->Jmol syncing and tweak UI (no more sparkles and better vocabulary).

Changed 7 years ago by gutow

Fix server<->Jmol syncing and tweak UI (no more sparkles and better vocabulary).

comment:87 Changed 7 years ago by gutow

Oops! Now there are two patches. Since the first one gave a trac error on upload, use patch 2. Although patch 1 looks OK by eye.

comment:88 Changed 7 years ago by kcrisman

  • Report Upstream changed from Reported upstream. No feedback yet. to Workaround found; Bug reported upstream.
  • Reviewers changed from Karl-Dieter Crisman, Steven Trogdon to Karl-Dieter Crisman, Steven Trogdon, Punarbasu Purkayastha

Don't worry about the error, that is this whole Trac overloading thing. They should be identical. Unfortunately, I can't view either of them right now, Trac is still acting up.

Good call on the scroll, ppurka; I just ignored it because I'm used to things like that in other software :) but we shouldn't be like them.

Jonathan, nice work on changing the defaults; things look great on the server. The change to opacity makes more sense as well. The other stuff with the deletes is presumably in comment:71, so I say positive review on that patch, now just need to do the doctest stuff.

comment:89 Changed 7 years ago by kini

I pushed the new patch to the pull request.

comment:90 follow-up: Changed 7 years ago by strogdon

Well, no joy here. I applied the new patch and locally I basically get the same behavior as reported in first scenario from comment:64 even though from the TestServer? things were fine. There is a caveat that indicates something has changed. Once I get the pop-up and click to remove it I have

Loading Jmol 3-D viewer...

in the present applet and there are 3 interactive applets and 1 that is sleeping. If the sleeping applet is made interactive things "seem" to be resolved, but I'm left with 2 sleeping applets and one of them cannot be make interactive.

I'm wondering if there is something server-side that's not predictable across platforms that's causing this. Let's see what Karl gets. I didn't mention previously but the version of FF I'm using is 13. I'll also try this later on an ubuntu machine.

comment:91 Changed 7 years ago by gutow

  • Description modified (diff)

comment:92 in reply to: ↑ 90 ; follow-up: Changed 7 years ago by gutow

Replying to strogdon:

Well, no joy here. I applied the new patch and locally I basically get the same behavior as reported in first scenario from comment:64 even though from the TestServer? things were fine. There is a caveat that indicates something has changed. Once I get the pop-up and click to remove it I have

Loading Jmol 3-D viewer...

in the present applet and there are 3 interactive applets and 1 that is sleeping. If the sleeping applet is made interactive things "seem" to be resolved, but I'm left with 2 sleeping applets and one of them cannot be make interactive.

I'm wondering if there is something server-side that's not predictable across platforms that's causing this. Let's see what Karl gets. I didn't mention previously but the version of FF I'm using is 13. I'll also try this later on an ubuntu machine.

Did you remember to to a ./sage -b to force recognition of the new code? Only suggesting it because I keep forgetting myself...

Changed 7 years ago by gutow

complete doctest coverage in jmoldata.py and increase JmolData?.jar memory

comment:93 Changed 7 years ago by gutow

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues doctests,switch to generic error deleted

Decided to keep JmolData specific error as may need it for movie generation.

Last edited 7 years ago by kini (previous) (diff)

comment:94 in reply to: ↑ 92 ; follow-up: Changed 7 years ago by strogdon

Replying to gutow:

Did you remember to to a ./sage -b to force recognition of the new code? Only suggesting it because I keep forgetting myself...

Hmm ..., well I thought I had? I even checked my history to make sure I had. But ... after running ./sage -b "again" - I now get results exactly as those from the TestServer! Sorry for the noise.

Last edited 7 years ago by kini (previous) (diff)

comment:95 in reply to: ↑ 94 ; follow-up: Changed 7 years ago by kcrisman

  • Status changed from needs_review to needs_work
  • Work issues set to very minor doc issues

Did you remember to to a ./sage -b to force recognition of the new code? Only suggesting it because I keep forgetting myself...

Hmm ..., well I thought I had? I even checked my history to make sure I had. But ... after running ./sage -b "again" - I now get results exactly as those from the TestServer! Sorry for the noise.

Yup, this happens to everybody! Great, though - I would have been surprised had this been different.

Decided to keep JmolData? specific error as may need it for movie generation.

Sorry, still 'needs work'. Every method needs doctesting. In particular, the JmolDataError needs a docstring and test in its __init__ and __str__ methods. I'm really sorry this is so annoying. For the init, just print self.value or something; you can also put # indirect doctest as a comment after the test (not the result) for the ones that don't use the actual name of the method.

Also, you'll need to change things like

EXAMPLES:: 
Create a JmolData object 
:: 
    sage: from sage.interfaces.jmoldata import JmolData 

should look like

EXAMPLES:

Create a JmolData object:: 

    sage: from sage.interfaces.jmoldata import JmolData 

Note the single colon except directly before the doctest, and the blank line before the actual test.

Will increasing the Jmol memory cause problems? I don't have time to do any more Jmol testing right now, how much more memory are we talking, where does it live?

comment:96 Changed 7 years ago by gutow

  • Description modified (diff)

comment:97 in reply to: ↑ 95 Changed 7 years ago by gutow

Replying to kcrisman:

Decided to keep JmolData? specific error as may need it for movie generation.

Sorry, still 'needs work'. Every method needs doctesting. In particular, the JmolDataError needs a docstring and test in its __init__ and __str__ methods.

No biggy. I hope to have time to get to it tomorrow. Have a research student in all day working on building a high-powered node (8 cores) for our quantum mechanics cluster.

Will increasing the Jmol memory cause problems? I don't have time to do any more Jmol testing right now, how much more memory are we talking, where does it live?

This just brings the memory used by the image generating application up to what is used by the applet in the browser (512 MB of heap--much can be virtual). Anyway, the image generating application should now be able to handle data sets at least as big as the applet. It should have no effect on the in browser memory issues.

Changed 7 years ago by gutow

more doctests plus formatting fixes

comment:98 Changed 7 years ago by gutow

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues very minor doc issues deleted

Think I've fixed all the doctests and formatting. Please check.

comment:99 Changed 7 years ago by kcrisman

There are a number of formatting issues, and one definite 'needs work' item (you create a file which isn't removed during doctesting).

Patch coming up.

Changed 7 years ago by kcrisman

comment:100 follow-up: Changed 7 years ago by kcrisman

  • Description modified (diff)

Okay, I've created a reviewer patch. Now I know the formatting is right because I added this to the reference manual and looked.

I also made some linebreaks earlier, made one set of tests optional because they require the internet (I've checked this), and made the file temporary that wasn't before.

So in theory, this should be wonderful. Only now I've horribly broken something, because these files in the os.path.exists tests no longer exist and those tests fail! I have precisely zero explanation; every time I made them live in the current directory, like just DNA.png, it worked, every time I put them someplace else it didn't.

What's particularly odd about this is that the other test which imports SAGE_TMP used to pass, and then started failing. For no discernible reason. So I'm wondering if I just destroyed something else on my computer (how, I don't know - I've just been working on this patch).

So 'needs review' still. I really am stumped, as this should be completely done now, but isn't.

On the plus side, it looks like people like the functionality!

comment:101 Changed 7 years ago by jhpalmieri

  • Description modified (diff)

comment:102 follow-up: Changed 7 years ago by jhpalmieri

For me, one os.path.exists test fails: line 164 in jmoldata.py. When I run it by hand, the file is not created. I edited jmoldata.py, removing stdout=jout in the subprocess.call line, and I see this when I try to run it by hand:

sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
Error occurred during initialization of VM
Incompatible minimum and maximum heap sizes specified

Does that help to track down the problem?

comment:103 in reply to: ↑ 100 Changed 7 years ago by gutow

Replying to kcrisman:

Okay, I've created a reviewer patch. Now I know the formatting is right because I added this to the reference manual and looked.

So in theory, this should be wonderful. Only now I've horribly broken something, because these files in the os.path.exists tests no longer exist and those tests fail! I have precisely zero explanation; every time I made them live in the current directory, like just DNA.png, it worked, every time I put them someplace else it didn't.

Your problem is in line 163. I ran into this when I was writing the test code. You have too many layers of escaping on the endofline. This is why I used the form:

sage: script = 'set defaultdirectory "'+archive_name+'"\n script SCRIPT\n' 

rather than the

sage: script = 'set defaultdirectory "%s"\\n script SCRIPT\\n'%archive_name

you used. Alternatively, your code works if you change it to:

sage: script = 'set defaultdirectory "%s"\n script SCRIPT\n'%archive_name

comment:104 in reply to: ↑ 102 Changed 7 years ago by gutow

Replying to jhpalmieri:

For me, one os.path.exists test fails: line 164 in jmoldata.py. When I run it by hand, the file is not created. I edited jmoldata.py, removing stdout=jout in the subprocess.call line, and I see this when I try to run it by hand:

sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
Error occurred during initialization of VM
Incompatible minimum and maximum heap sizes specified

Does that help to track down the problem?

Thanks for reporting this. I cannot reproduce this error once I fix the double backslash issue in the tests. However, it suggests I should to be more careful about how I specify the memory allocation for the JVM. I will set a maximum, rather than a minimum.

Changed 7 years ago by gutow

fix backslash issues and define maximum heap size for JVM

comment:105 Changed 7 years ago by gutow

  • Description modified (diff)

comment:106 follow-up: Changed 7 years ago by jhpalmieri

This passes tests for me. Regarding jmol_scratch: should this be a temporary directory, or is it important that it persists from session to session? If it should persist, then ~/.sage/ should be replaced by DOT_SAGE, for example like this:

  • sage/interfaces/jmoldata.py

    diff --git a/sage/interfaces/jmoldata.py b/sage/interfaces/jmoldata.py
    a b AUTHORS: 
    2222from sage.structure.sage_object import SageObject
    2323
    2424from sage.misc.misc import tmp_filename
    25 from sage.misc.misc import SAGE_LOCAL
     25from sage.misc.misc import SAGE_LOCAL, DOT_SAGE, sage_makedirs
    2626
    2727import subprocess
    2828import os
    class JmolData(SageObject): 
    8181            <type 'bool'>
    8282        """
    8383        #scratch file for  Jmol errors and status
    84         jmolscratch = os.path.expanduser("~/.sage/sage_notebook.sagenb/jmol_scratch")
     84        jmolscratch = os.path.join(DOT_SAGE, "sage_notebook.sagenb", "jmol_scratch")
    8585        if not os.path.exists(jmolscratch):
    86             os.mkdir(jmolscratch)
     86            sage_makedirs(jmolscratch)
    8787        scratchout = os.path.join(jmolscratch,"jmolout.txt")
    8888        jout=open(scratchout,'w')
    89         testjavapath = os.path.join(SAGE_LOCAL, "share/jmol/testjava.sh")
     89        testjavapath = os.path.join(SAGE_LOCAL, "share", "jmol", "testjava.sh")
    9090        result = subprocess.call([testjavapath],stdout=jout)
    9191        jout.close()
    9292        if (result == 0):
    class JmolData(SageObject): 
    168168         """
    169169        if (self.is_jvm_available()):
    170170            # Set up paths, file names and scripts
    171             jmolpath = os.path.join(SAGE_LOCAL, "share/jmol/JmolData.jar")
     171            jmolpath = os.path.join(SAGE_LOCAL, "share", "jmol", "JmolData.jar")
    172172            launchscript = ""
    173173            if (datafile_cmd!='script'):
    174174                launchscript = "load "
    class JmolData(SageObject): 
    179179
    180180            sizeStr = "%sx%s" %(figsize*100,figsize*100)
    181181            #scratch file for  Jmol errors and status
    182             jmolscratch = os.path.expanduser("~/.sage/sage_notebook.sagenb/jmol_scratch")
     182            jmolscratch = os.path.join(DOT_SAGE, "sage_notebook.sagenb", "jmol_scratch")
    183183            if not os.path.exists(jmolscratch):
    184                 os.mkdir(jmolscratch)
     184                sage_makedirs(jmolscratch)
    185185            scratchout = os.path.join(jmolscratch,"jmolout.txt")
    186186            jout=open(scratchout,'w')
    187187            #now call the java application and write the file.

Otherwise, you could use SAGE_TMP so that it would be cleaned up when Sage exits.

comment:107 in reply to: ↑ 106 Changed 7 years ago by gutow

  • Status changed from needs_review to needs_work
  • Work issues set to use predefined paths

Replying to jhpalmieri:

This passes tests for me. Regarding jmol_scratch: should this be a temporary directory, or is it important that it persists from session to session? If it should persist, then ~/.sage/ should be replaced by DOT_SAGE, for example like this:

I think the file should be permanent because it shows the last few console lines associated with launches of the JmolData?.jar process or looking for the JVM. I was thinking of the file as a size limited log file that can be used to trace problems associated with JmolData?.jar.

I like your suggestion. Sage is big enough that I find I have missed many predefined values such as DOT_SAGE.

comment:108 Changed 7 years ago by jhpalmieri

I'm attaching my suggestion as an actual patch, if you just want to use it as is.

Changed 7 years ago by jhpalmieri

minor cleanup of paths in jmoldata.py

comment:109 Changed 7 years ago by gutow

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Thanks, saved me making it into a patch. I did verify that the changes work fine on my system too.

Back to needs_review. Any takers?

comment:110 Changed 7 years ago by kcrisman

The reason I did the \\n is because the \n breaks building the documentation. I figured that was the culprit on the testing. I guess we could just not build it, but that seems silly. So with attachment:trac_12299_test_typos_JVM_mem.patch applied, it breaks. I'm attaching a screenshot.

jhpalmieri, you seems to have become the Sphinx bloodhound - is there any way to fix both of these problems simultaneously?

Changed 7 years ago by kcrisman

Shows documentation not building properly

comment:111 Changed 7 years ago by kcrisman

In fact, I'm surprised you didn't see

/Users/.../sage-5.1.beta1-flask/local/lib/python2.7/site-packages/sage/interfaces/jmoldata.py:docstring of sage.interfaces.jmoldata.JmolData.export_image:34: WARNING: Block quote ends without a blank line; unexpected unindent.
/Users/.../sage-5.1.beta1-flask/local/lib/python2.7/site-packages/sage/interfaces/jmoldata.py:docstring of sage.interfaces.jmoldata.JmolData.export_image:35: WARNING: Block quote ends without a blank line; unexpected unindent.

See the "Screen shot 2012-06-18 at 8.28.42 AM.png" attachment (can't get a link to attachments with spaces in name) for what it looks like.

comment:112 Changed 7 years ago by kcrisman

  • Reviewers changed from Karl-Dieter Crisman, Steven Trogdon, Punarbasu Purkayastha to Karl-Dieter Crisman, Steven Trogdon, Punarbasu Purkayastha, John Palmieri
  • Status changed from needs_review to needs_work
  • Work issues changed from use predefined paths to doc built right, update pull request

Incidentally, John's patch looks good, especially since sometimes DOT_SAGE is indeed set differently.

Jonathan, what would you say still needs review? The user interface piece certainly seems to be in good shape, based on strogdon, ppurka, and my testing.

I guess once this is ready we'd need an updated pull request; kini has graciously offered to do this, I think.

comment:113 Changed 7 years ago by kini

Looking at the ticket description, I only see three patches for sagenb, and all the rest appear to be for the sage library - no? And there are already three patches on the pull request... what needs updating?

comment:114 follow-up: Changed 7 years ago by jhpalmieri

This one character patch should fix the docbuilding.

  • sage/interfaces/jmoldata.py

    diff --git a/sage/interfaces/jmoldata.py b/sage/interfaces/jmoldata.py
    a b class JmolData(SageObject): 
    101101        image_type ='PNG', #PNG, JPG, GIF
    102102        figsize=5,
    103103        **kwds):
    104         """
     104        r"""
    105105        This executes JmolData.jar to make an image file.
    106106
    107107        INPUT:

comment:115 Changed 7 years ago by jhpalmieri

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues doc built right, update pull request deleted

comment:116 Changed 7 years ago by jhpalmieri

  • Description modified (diff)

comment:117 in reply to: ↑ 114 Changed 7 years ago by kcrisman

This one character patch should fix the docbuilding.

Duh. You wouldn't believe how many variants of r in the actual set default directory script I tried; that the issue was the entire docstring somehow didn't occur to me. Thank you very much.

There is still

Image file , .png, .gif or .jpg (default .png) 

but removing the space or the comma or anything now really seems too trivial to deal with.

Sorry, kini, I didn't realize that the only remaining updates were on the sage library end.

Well, I don't know how much is really left to review here. gutow, let us know what you think might not have been looked at carefully enough.

Changed 7 years ago by jhpalmieri

all Sage library patches, folded into one

comment:118 Changed 7 years ago by kcrisman

I like the Sage library patch. We've certainly tested all of it a fair amount as well.

I don't have access to a computer without JVM, I guess, so maybe someone should make sure the error message really appears in the notebook and is friendly?

The tests now behave properly again.

Line 2416 could use an os.path.join, I guess, but that's probably not crucial.

I can't possibly review all the changes in jmol_lib.js but it certainly mostly conforms to the changes that are advertised, and I believe strogdon and I have tested a fair number of those capabilities. Is someone willing to read through the changes in that file line by line to look for errors?

I honestly don't know that we're going to find a lot more without the kind of testing one gets from active deployment. So far it seems like it's performing admirably. Other comments?

comment:119 Changed 7 years ago by ppurka

This looks good.

I tested this via your website on a Chromium 18.0.1025.151 (Developer Build 130497 Linux) Ubuntu 10.04 x86 system with Oracle Java 7 Java(TM) Plug-in 1.7.0_03. Initially it tried to reload the jmol a second time, but after that it worked good.

comment:120 Changed 7 years ago by gutow

I'm inclined to agree with Karl-Dieter. I think that most of the bugs are now going to be found through usage. I will continue to work on cleaning up the code and trying to update things to take advantage of new Jmol capabilities as they come along.

I have checked the fallback on a server without a JVM about a month ago. The section of code that falls back to Tachyon hasn't changed since then, so I think we are OK. However, if someone else has a machine they could check that on I would appreciate it. The one I used to check is no longer available. A JVM got installed on it ;).

Before a last approval I would like to see someone other than me (the author) verify that the current instructions work to give a functional installation of this ticket. If somebody can do that, I cannot think of anything else that still needs testing.

comment:121 Changed 7 years ago by kcrisman

  • Milestone changed from sage-5.2 to sage-pending
  • Report Upstream changed from Workaround found; Bug reported upstream. to Fixed upstream, in a later stable release.
  • Status changed from needs_review to positive_review

I just updated my Java on Mac and other than having to close the browser window when Safari asked me to enable the Java plugin or something, no problems.

Following instructions on #11080 and then here leads to success on a Sage that didn't have it before. I accidentally already had jmoldata.py in there so I had to do a little stuff to cover that up, but that didn't make a difference. Certainly John making the all-in-one patch helped a lot. Important - the other dependencies are already in #11080!

I'm putting this to sage-pending until we know which sagenb version this will go in. Hopefully one included in Sage 5.2, but perhaps not - up to Keshav and Jeroen.

comment:122 Changed 7 years ago by kcrisman

  • Dependencies changed from #11080,#11078,#11503 to #11080,#11078,#11503,#13121
  • Description modified (diff)
  • Milestone changed from sage-pending to sage-5.2

According to this github comment we should expect this to be involved in #13121, sagenb 0.9.1.

Release manager - note that this has both sage and sagenb patches, it's not wholly upstream.

comment:123 Changed 7 years ago by kcrisman

This has been merged into the sagenb upstream.

comment:124 Changed 7 years ago by kini

  • Summary changed from Advance Jmol Interactive Features in Flask Notebook to Upgrade Jmol to 12.3.27, Advance Jmol Interactive Features in Flask Notebook

comment:125 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.2 to sage-pending

comment:126 Changed 7 years ago by jhpalmieri

  • Status changed from positive_review to needs_work

Running on a Mac, remotely logged in, I see this error when doctesting:

sage -t --long -force_lib "devel/sage/sage/interfaces/jmoldata.py"
Exception in thread "main" java.lang.InternalError: Can't connect to window server - not enough permissions.
	at java.lang.ClassLoader$NativeLibrary.load(Native Method)
	at java.lang.ClassLoader.loadLibrary0(ClassLoader.java:1827)
	at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1724)
	at java.lang.Runtime.loadLibrary0(Runtime.java:823)
	at java.lang.System.loadLibrary(System.java:1045)
	at sun.security.action.LoadLibraryAction.run(LoadLibraryAction.java:50)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.awt.image.ColorModel.loadLibraries(ColorModel.java:188)
	at java.awt.image.ColorModel.<clinit>(ColorModel.java:196)
	at org.jmol.awt.Image.<clinit>(Unknown Source)
	at org.jmol.awt.Platform.newBufferedRgbImage(Unknown Source)
	at org.jmol.g3d.Platform3D.allocateOffscreenImage(Unknown Source)
	at org.jmol.g3d.Platform3D.createInstance(Unknown Source)
	at org.jmol.g3d.Graphics3D.<init>(Unknown Source)
	at org.jmol.viewer.Viewer.<init>(Unknown Source)
	at org.jmol.viewer.Viewer.allocateViewer(Unknown Source)
	at org.jmol.api.JmolViewer.allocateViewer(Unknown Source)
	at org.openscience.jmol.app.JmolData.<init>(Unknown Source)
	at org.openscience.jmol.app.JmolData.main(Unknown Source)
**********************************************************************
File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/6495plus13121/sage-5.1.rc0-6495/devel/sage/sage/interfaces/jmoldata.py", line 165:
    sage: print os.path.exists(testfile)
Expected:
    True
Got:
    False
**********************************************************************

When I'm sitting at the same machine, I see JMol open up as a separate application; perhaps there is a problem doing this when remotely logged in.

comment:127 Changed 7 years ago by gutow

This is not really a Java security error. It appears that something happened to the code allowing Jmol to launch without a GUI (aka windowserver). Looking at the patches it appears that somewhere along the line the necessary option in the Java launch command got lost. I will put up a patch as soon as I double check the syntax.

Good catch!

Changed 7 years ago by gutow

reinserting headless option for java launch

comment:128 Changed 7 years ago by gutow

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I think this patch will solve the remote login problem. It does allow the sage server to run Jmol even if the server is running as a user without a GUI.

comment:129 Changed 7 years ago by jhpalmieri

  • Status changed from needs_review to positive_review

Looks good to me, and tests now pass.

comment:130 follow-up: Changed 7 years ago by jhpalmieri

For anyone involved in this ticket: have you tried this version of Jmol with interacts, for example the ones on the wiki? It always seems to default to "Sleeping". Maybe that's always true, not just for interacts? And for all plots, there is also the "mesh" issue that I think has been mentioned elsewhere: passing mesh=True to plot3d has no effect on Jmol's display.

comment:131 Changed 7 years ago by kcrisman

It always seems to default to "Sleeping". Maybe that's always true, not just for interacts?

This is discussed a fair amount above. I still disagree with the decision, but I understand Jonathan's reluctance to allow it. I'm sure he'd be happy to explain the syncing issues he's worried about again :)

passing mesh=True to plot3d has no effect on Jmol's display.

There is an open ticket about keywords to plot3d not being passed to show in general, and is unrelated to this afaik.

comment:132 in reply to: ↑ 130 Changed 7 years ago by gutow

Replying to jhpalmieri:

For anyone involved in this ticket: have you tried this version of Jmol with interacts, for example the ones on the wiki? It always seems to default to "Sleeping". Maybe that's always true, not just for interacts?

As Karl-Dieter says, I have made the default sleeping for all recalculations of 3-D objects done by Sage because Sage and the Notebook are presently set up to relaunch Jmol each time this is done rather than just pass the new data to an existing applet. This means that if you do not have a very good network connection you eventually get into a situation where the browser is trying to load more than one applet to the same location. This causes hangs. In the case of interacts that generate an update each time an interactive control is touched this happens quite easily. Until I or somebody else has time to rewrite the the way applets or any other 3-D rendering is handled this is the only safe option. An intermediate, I hope to have time to work on first, is a spinning (animated GIF) instead of just the static 3-D image. This will have to wait until I get my quantum calculation cluster running (28 CPUs are up, plan to add about 4 more) and do all the glassblowing repairs necessary for my lab class this fall.

And for all plots, there is also the "mesh" issue that I think has been mentioned elsewhere: passing mesh=True to plot3d has no effect on Jmol's display.

I'm aware of this, but have not looked into it yet. There is a ticket. It isn't just "mesh", there are number of things that don't get passed.

comment:133 Changed 7 years ago by jdemeyer

  • Status changed from positive_review to needs_info

The installation notes are a mess, please clarify, mentioning only the things relevant to this ticket.

Last edited 7 years ago by jdemeyer (previous) (diff)

comment:134 Changed 7 years ago by jdemeyer

Also, the Changelog in SPKG.txt doesn't make much sense. There is a jmol-12.2.21.p4 mentioned, while #11503 only has jmol-12.2.21.p0. This should be fixed.

Also, hg log shows only a single commit. This isn't really a problem, but as advice for the future, I strongly recommend to have at least a separate commit for every released version.

comment:135 Changed 7 years ago by kcrisman

Jonathan, I wonder how hard it would be (perhaps not here) to enable a switch so that at the top of a page or something one could call "full-evaluate-jmol" or something and then Jmols would actually not default to sleeping, if one figured one was in a good situation for that. I'm feeling like a lot of the WeBWorK embeddings and Sage cell interacts, among others, would really suffer with this... or maybe not. As I said, perhaps not here.

comment:136 Changed 7 years ago by gutow

Definitely not here. I suggest a ticket (or what ever) in the notebook on git. This will require rewriting the exact format of a cell in the notebook. I haven't got time to work on that now, but if we can find some people who do, I could serve in an advisory roll.

comment:137 Changed 7 years ago by gutow

  • Description modified (diff)

I've tried to make the instructions clearer and cleaner. Are they now OK?

I will look into the #11503 spkg.txt issue. Probably a typo...

comment:138 Changed 7 years ago by jdemeyer

  • Description modified (diff)

I made the instructions even more clear according to my interpretation of them. Are they correct?

Yes, please have a look at SPKG.txt and set the ticket status back to positive_review once you've figured that out and fixed the spkg.

comment:139 follow-up: Changed 7 years ago by jdemeyer

On arando, I get the doctest failure:

sage -t  --long -force_lib devel/sage/sage/interfaces/jmoldata.py
**********************************************************************
File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 164:
    sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
Exception raised:
    Traceback (most recent call last):
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_6[14]>", line 1, in <module>
        JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")###line 164:
    sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/lib/python/site-packages/sage/interfaces/jmoldata.py", line 195, in export_image
        raise JmolDataError(errStr)
    JmolDataError: 'Java Virtual Machine not available.\nThis should be checked before calling JmolData().export_image().\nUse JmolData().is_jvm_available() to check.\nAdministrator should install JVM.'
**********************************************************************
File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 165:
    sage: print os.path.exists(testfile)
Expected:
    True
Got:
    False
**********************************************************************

On rosemary, the doctest failure

sage -t  --long -force_lib devel/sage/sage/interfaces/jmoldata.py

(.:323): Gtk-WARNING **: cannot open display:  
**********************************************************************
File "/home/buildbot/build/sage/rosemary-1/rosemary_full/build/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 165:
    sage: print os.path.exists(testfile)
Expected:
    True
Got:
    False
**********************************************************************

comment:140 in reply to: ↑ 139 ; follow-up: Changed 7 years ago by gutow

The first is the expected error on machines without JVMs installed. I had trouble writing doctests for this because of the multiple possible outcomes. Could we rewrite the doctests to output something like: "JVM was/was not detected, thus an image was/was not generated?" If I understand the doctest mechanism correctly it is flexible enough to check for just the "JVM was..." part, right? Then it could produce some other text if the JVM was found, but image generation failed. This last case would be the only failure mode. Would that be better?

The second one looks as if you did not get this patch installed: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299_headless_java.patch

Replying to jdemeyer:

On arando, I get the doctest failure:

sage -t  --long -force_lib devel/sage/sage/interfaces/jmoldata.py
**********************************************************************
File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 164:
    sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
Exception raised:
    Traceback (most recent call last):
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_6[14]>", line 1, in <module>
        JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")###line 164:
    sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
      File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/local/lib/python/site-packages/sage/interfaces/jmoldata.py", line 195, in export_image
        raise JmolDataError(errStr)
    JmolDataError: 'Java Virtual Machine not available.\nThis should be checked before calling JmolData().export_image().\nUse JmolData().is_jvm_available() to check.\nAdministrator should install JVM.'
**********************************************************************
File "/var/lib/buildbot/build/sage/arando-1/arando_full/build/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 165:
    sage: print os.path.exists(testfile)
Expected:
    True
Got:
    False
**********************************************************************

On rosemary, the doctest failure

sage -t  --long -force_lib devel/sage/sage/interfaces/jmoldata.py

(.:323): Gtk-WARNING **: cannot open display:  
**********************************************************************
File "/home/buildbot/build/sage/rosemary-1/rosemary_full/build/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 165:
    sage: print os.path.exists(testfile)
Expected:
    True
Got:
    False
**********************************************************************

comment:141 in reply to: ↑ 140 ; follow-up: Changed 7 years ago by jdemeyer

Replying to gutow:

The second one looks as if you did not get this patch installed: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299_headless_java.patch

That patch was applied.

comment:142 in reply to: ↑ 141 ; follow-up: Changed 7 years ago by gutow

Odd, because that patch causes Jmol to run without asking for a display. The other oddity is that I wouldn't expect an error from GTK unless the machine has a GUI installed. If it does have a GUI installed than Jmol should be able to open a GUI display, although it shouldn't be trying. Which JVM is installed? Replying to jdemeyer:

Replying to gutow:

The second one looks as if you did not get this patch installed: http://trac.sagemath.org/sage_trac/raw-attachment/ticket/12299/trac_12299_headless_java.patch

That patch was applied.

comment:143 in reply to: ↑ 142 ; follow-up: Changed 7 years ago by jdemeyer

Replying to gutow:

Which JVM is installed?

I have no idea, tell me how to check.

comment:144 in reply to: ↑ 143 Changed 7 years ago by gutow

The command is: "java -version" All the information it provides might be useful.

Replying to jdemeyer:

Replying to gutow:

Which JVM is installed?

I have no idea, tell me how to check.

comment:145 follow-up: Changed 7 years ago by jdemeyer

buildbot@rosemary ~$ java -version
java version "1.4.2"
gij (GNU libgcj) version 4.1.2 20080704 (Red Hat 4.1.2-50)

Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

comment:146 in reply to: ↑ 145 Changed 7 years ago by gutow

Replying to jdemeyer:

java version "1.4.2"

I'll poke around a little but this version of Java began phase out in 2006 and was deprecated in 2008. Most of Jmol does work under that version, but I'm not sure headless operations can be made to work. Is this going to be a holdup?

I won't have significant bandwidth until Thursday, so the spkg with the typo fix in SPKG.txt will not be put up until then.

comment:147 follow-up: Changed 7 years ago by strogdon

I have access to this very, very old redhat machine for which:

[strogdon@bwulf sage-5.2.beta0]$ java -version
java version "1.4.2"
Java(TM) 2 Runtime Environment, Standard Edition (build 2.2)
IBM J9SE VM (build 2.2, J2RE 1.4.2 IBM J9 2.2 Linux amd64-64 j9xa64142-20041210 (JIT enabled)
J9VM - 20041208_2100_LHdSMr
JIT  - r7_level20041130_1809)

and when I run the testsuite the only failing test is

sage -t -long -force_lib "devel/sage-main/sage/interfaces/jmoldata.py"
The java class could not be loaded. java.lang.UnsupportedClassVersionError: (org/openscience/jmol/app/JmolData) bad major version at offset=6
**********************************************************************
File "/data/sage/sage-5.2.beta0/devel/sage-main/sage/interfaces/jmoldata.py", line 165:
    sage: print os.path.exists(testfile)
Expected:
    True
Got:
    False
**********************************************************************

So perhaps the Gtk-WARNING is unrelated to java version.

comment:148 Changed 7 years ago by kini

  • Description modified (diff)

Edited the description to not finalize queues because it's a pain to remove commits from a Sage install if you decide you want to test some other ticket after this one.

Also edited in a pointer to #13121 for installing this on >sage-5.1.

Last edited 7 years ago by kini (previous) (diff)

comment:149 Changed 7 years ago by kini

  • Description modified (diff)

comment:150 in reply to: ↑ 147 Changed 7 years ago by gutow

Replying to strogdon:

Would you happen to have access to a machine running Java 1.5? I think maybe I should put in a test for the Java version and I will need to double check which versions fail. Also wondering if the Java version checking should be a separate ticket? Any thoughts?

comment:151 Changed 7 years ago by jdemeyer

Here are various java versions of machines that I have access to:

buildbot@boxen ~$ java -version
java version "1.5.0"
gij (GNU libgcj) version 4.2.4 (Ubuntu 4.2.4-1ubuntu3)

Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
buildbot@sage ~$ java -version
java version "1.6.0_24"
Java(TM) SE Runtime Environment (build 1.6.0_24-b07)
Java HotSpot(TM) 64-Bit Server VM (build 19.1-b02, mixed mode)
moufang:~ buildbot$ java -version
java version "1.5.0_19"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_19-b02-306)
Java HotSpot(TM) Client VM (build 1.5.0_19-138, mixed mode, sharing)
buildbot@hawk:~$ java -version
java version "1.6.0_18"
Java(TM) SE Runtime Environment (build 1.6.0_18-b07)
Java HotSpot(TM) Server VM (build 16.0-b13, mixed mode)
buildbot@rosemary ~$ java -version
java version "1.4.2"
gij (GNU libgcj) version 4.1.2 20080704 (Red Hat 4.1.2-50)

Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
buildbot@arando:~$ java -version
The program 'java' can be found in the following packages:
 * default-jre
 * gcj-4.6-jre-headless
 * openjdk-6-jre-headless
 * gcj-4.5-jre-headless
 * openjdk-7-jre-headless
Ask your administrator to install one of them
buildbot@snapperkob:~$ java -version
java version "1.6.0_24"
OpenJDK Runtime Environment (IcedTea6 1.11.1) (6b24-1.11.1-4ubuntu3)
OpenJDK 64-Bit Server VM (build 20.0-b12, mixed mode)
buildbot@bsd ~$ java -version
java version "1.6.0_33"
Java(TM) SE Runtime Environment (build 1.6.0_33-b03-424-10M3720)
Java HotSpot(TM) 64-Bit Server VM (build 20.8-b03-424, mixed mode)

comment:152 follow-up: Changed 7 years ago by gutow

So I think you've answered my question. The failures occurred on arando (no java, failure expected) and rosmary (Java 1.4.2), so I need to test for java >=1.5. Testing for this will require changes to the Jmol spkg. Could we make that another ticket? I don't think it will take me very long, but I don't want to hold things up or have to start the review process for this ticket again.

comment:153 in reply to: ↑ 152 ; follow-up: Changed 7 years ago by jdemeyer

Replying to gutow:

Could we make that another ticket? I don't think it will take me very long, but I don't want to hold things up or have to start the review process for this ticket again.

I would do it on this ticket. That doesn't mean the whole ticket with all its patches needs to be reviewed again, only the incremental change with the java version check.

I remind you also to fix the Changelog in SPKG.txt.

comment:154 in reply to: ↑ 153 Changed 7 years ago by gutow

Replying to jdemeyer:

Replying to gutow:

I would do it on this ticket. That doesn't mean the whole ticket with all its patches needs to be reviewed again, only the incremental change with the java version check.

OK, I will have access to enough bandwidth to upload fixes tomorrow.

I remind you also to fix the Changelog in SPKG.txt.

Already done, will be in tomorrow's upload.

comment:155 Changed 7 years ago by kini

  • Description modified (diff)

comment:156 Changed 7 years ago by gutow

  • Description modified (diff)
  • Status changed from needs_info to needs_review

Fixes typo in SPKG.txt Adds check for version 1.5 -1.7 of Java.

comment:157 Changed 7 years ago by gutow

With the added version checking doctesting should now fail on both arando (no java) and rosemary (deprecated java).

comment:158 follow-up: Changed 7 years ago by strogdon

On this old redhat machine with jmol-12.3.27.p1.spkg I get a failure as on arando:

sage -t -long -force_lib "devel/sage-main/sage/interfaces/jmoldata.py"
**********************************************************************
File "/data/sage/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 164:
    sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
Exception raised:
    Traceback (most recent call last):
      File "/data/sage/sage-5.2.beta1/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/data/sage/sage-5.2.beta1/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/data/sage/sage-5.2.beta1/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_6[14]>", line 1, in <module>
        JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")###line 164:
    sage: JData.export_image(targetfile =testfile,datafile = script, image_type="PNG")
      File "/data/sage/sage-5.2.beta1/local/lib/python/site-packages/sage/interfaces/jmoldata.py", line 195, in export_image
        raise JmolDataError(errStr)
    JmolDataError: 'Java Virtual Machine not available.\nThis should be checked before calling JmolData().export_image().\nUse JmolData().is_jvm_available() to check.\nAdministrator should install JVM.'
**********************************************************************
File "/data/sage/sage-5.2.beta1/devel/sage-main/sage/interfaces/jmoldata.py", line 165:
    sage: print os.path.exists(testfile)
Expected:
    True
Got:
    False
**********************************************************************

The JmolDataError? output is not strictly correct (JVM is installed but is deprecated), but this may be cosmetic.

comment:159 Changed 7 years ago by strogdon

It seems the java tests depend on running an external script "testjava.sh". I believe what's done in that script can be moved into jmoldata.py by locating the system bash:

sage: from twisted.python.procutils import which
sage: which("bash")[0]
'/bin/bash'

and then running subprocess.call() with it. Of course twisted is needed. Is this desirable? Which approach is easier to maintain?

comment:160 Changed 7 years ago by jhpalmieri

I think that the first line of testjava.sh should be #!/usr/bin/env bash instead of #!/bin/bash.

If we want to do it in Python instead, then rather than messing around with twisted.python.procutils.which (we shouldn't use twisted for this), we could do:

import re, subprocess

try:
    version = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT)
    return re.search("version.*[1]\.[567]", version)
except (subprocess.CalledProcessError, OSError):
    return False

comment:161 follow-up: Changed 7 years ago by ppurka

return re.search("version.*[1]\.[567]", version)

maybe make this line

return re.search("version.*[1]\.[5-9]", version)

to make it future proof. :)

comment:162 in reply to: ↑ 161 ; follow-up: Changed 7 years ago by gutow

No. I don't think we should accept versions the tool has not been checked against. There is good precedence. We are not running the latest python.

Replying to ppurka:

return re.search("version.*[1]\.[567]", version)

maybe make this line

return re.search("version.*[1]\.[5-9]", version)

to make it future proof. :)

comment:163 Changed 7 years ago by gutow

I tried to do the checking all in python originally. It was not working reliably. I still think it should be possible. However, I believe that effort (even if minor) should be a new ticket, as we are at the point where we may hold up inclusion of what already works in favor of improvements which do not change the actual functionality.

comment:164 in reply to: ↑ 162 ; follow-up: Changed 7 years ago by ddrake

Replying to gutow:

No. I don't think we should accept versions the tool has not been checked against.

I agree. We should only test for versions that we know will work. I also think we should keep the shell script for now; if we think re-doing it in Python is really important, we can do that later.

What needs to be reviewed to get this in? This ticket is holding up a big group of important tickets and I really want to get it merged.

comment:165 in reply to: ↑ 164 Changed 7 years ago by strogdon

Replying to ddrake:

What needs to be reviewed to get this in? This ticket is holding up a big group of important tickets and I really want to get it merged.

I would certainly give it positive review but thought I would wait for the "strange" Gtk failure on rosemary. But maybe that's not important.

comment:166 follow-ups: Changed 7 years ago by ddrake

John Palmieri told me that the testjava.sh shell script is one of the holdups, and it's not working properly for me on a machine with no Java installed.

The doctest uses subprocess.call, which returns the return code of the executed program. But testjava.sh isn't exiting with useful return codes. Should the script look like this?

#!/usr/bin/env bash

# is Java even installed?
type -atp java
if [ $? -gt 0 ]
then
    echo Java does not appear to be installed. Exiting.
    exit 1
fi

# we support versions 1.5 to 1.7
java -version 2>&1 | grep -q version.*[1]\.[567]
if [ $? -gt 0 ]
then
    echo You do not have a supported version of Java. Exiting.
    exit 1
fi

echo Java is available and is a supported version.

Then it gives informative error messages and exits with the proper exit codes.

comment:167 in reply to: ↑ 166 Changed 7 years ago by gutow

Replying to ddrake:

If you just run the shell script you will get no output as the result goes to the shell result variable. The subprocess.call picks them up fine. If you want to run the script by hand and see the codes you need to uncomment the last two lines to make the script look like below. I don't think we want this script returning long text messages. So that's why I've stripped them out. I also do not think we want to start with #!/usr/bin/env bash because that is not the standard way of prefacing most shell scripts that I've seen. I'm not fully clear on the difference, but since this script is launching as its own process, I want it to behave as any independent process on the computer.

#!/bin/bash
type -atp java
OUT=$?
#echo $OUT
if [ $OUT -eq 0 ]; then
#found java now check 1.5<=version<=1.7
#version >1.7 may be OK just not checked yet.
  java -version 2>&1|grep version.*[1]\.[567]
#else
#  echo "1"
fi
OUT=$?
echo $OUT

Although scripts default to retuning the output value of the last command run, we could put in explicit exit # lines if we want.

comment:168 in reply to: ↑ 166 Changed 7 years ago by gutow

Replying to ddrake:

java -version 2>&1 | grep -q version.*[1]\.[567]

The grep manpages strongly recommend against using -q and -s because they are not implemented on all systems and vary significantly from version to version. Although, if it works that helps to speed up script execution.

comment:169 Changed 7 years ago by gutow

I think the shell script with the added version checking and the typo related to patch level of previous versions of the spkg in SPKG.txt were the last issues.

comment:170 follow-up: Changed 7 years ago by ppurka

The #!/usr/bin/env bash syntax is recommended for portability.

As for the bash script, are you looking at the output of the bash script or only at the return status. In the former case, you probably want to echo back only the $OUT. In this case, the script can be shortened significantly

#!/usr/bin/env bash
type -atp java > /dev/null && java -version 2>&1 | grep version.*[1]\.[567] > /dev/null
echo $?

EDIT: Sorry, sage doesn't ship bash. I was recalling something incorrectly (mixed it up with bzip2).

Last edited 7 years ago by ppurka (previous) (diff)

comment:171 in reply to: ↑ 170 Changed 7 years ago by gutow

Replying to ppurka:

The #!/usr/bin/env bash syntax is recommended for portability.

I worry about all the caveats for failures and race conditions that are also mentioned in this link. I'm not sure it is an improvement, but am no expert.

As for the bash script, are you looking at the output of the bash script or only at the return status. In the former case, you probably want to echo back only the $OUT. In this case, the script can be shortened significantly

#!/usr/bin/env bash
type -atp java > /dev/null && java -version 2>&1 | grep version.*[1]\.[567] > /dev/null
echo $?

This short version looks like it works properly on a system with java 1.6 needs tests on a system without java and with 1.4. To completely minimize it the last "echo" could be commented out as well.

comment:172 follow-up: Changed 7 years ago by ddrake

Replying to ppurka:

The #!/usr/bin/env bash syntax is recommended for portability.

As for the bash script, are you looking at the output of the bash script or only at the return status.

There's a doctest in sage/interfaces/jmoldata.py (line 90) that looks like this:

result = subprocess.call([testjavapath],stdout=jout)

The documentation for subprocess.call says it returns the returncode attribute, but the current script AFAICT always returns 0. I think we should be using check_output if we want the output. We can do either thing, but it seems like right now the script and the doctest are not consistent.

In this case, the script can be shortened significantly

#!/usr/bin/env bash
type -atp java > /dev/null && java -version 2>&1 | grep version.*[1]\.[567] > /dev/null
echo $?

That's certainly an option, but perhaps the verbose-but-obvious script would be easier to maintain, in case someone who's not much of a bash expert needs to change it. :)

comment:173 Changed 7 years ago by jhpalmieri

I wasn't paying enough attention earlier. Since we're already using subprocess, I would suggest this color for the bike shed:

  • sage/interfaces/jmoldata.py

    diff --git a/sage/interfaces/jmoldata.py b/sage/interfaces/jmoldata.py
    a b class JmolData(SageObject): 
    8787        scratchout = os.path.join(jmolscratch,"jmolout.txt")
    8888        jout=open(scratchout,'w')
    8989        testjavapath = os.path.join(SAGE_LOCAL, "share", "jmol", "testjava.sh")
    90         result = subprocess.call([testjavapath],stdout=jout)
     90        try:
     91            version = subprocess.check_output(['java', '-version'], stderr=subprocess.STDOUT)
     92            import re
     93            result = bool(re.search("version.*[1]\.[567]", version))
     94        except (subprocess.CalledProcessError, OSError):
     95            result = False
    9196        jout.close()
    92         if (result == 0):
    93             return (True)
    94         else:
    95             return (False)
     97        return result
    9698
    9799    def export_image(self,
    98100        targetfile,
Last edited 7 years ago by jhpalmieri (previous) (diff)

comment:174 Changed 7 years ago by gutow

Replying to ddrake:

the current script AFAICT always returns 0. I think we should be using check_output if we want the output. We can do either thing, but it seems like right now the script and the doctest are not consistent.

1) The current script returns a result code of 1 (and so should ppurka's short script) if the java command is not available or if the version is not 1.5.x, 1.6,x or 1.7.x. This is the desired behavior for use by the jmoldata.py function that tests for the proper JVM.

2) subprocess.check_output raises an error of its own. Since there are a number of other kinds of errors I also expect to be specific to jmoldata.py functionality, I prefer testing the result code myself before generating jmoldata.py specific errors.

3) Ultimately, I hope this discussion is moot, as I would like to test directly inside python, using syntax similar to that suggested in comment:160 and comment:173. However, my objection to using the subprocess.CalledProcessError still stands.

comment:175 Changed 7 years ago by strogdon

I've tried something very similar to comment:173 here and it works for the tested java versions of 1.5 and 1.6. The only thing I see that's missing is appropriate writes to jmolout.txt to mimic the original jmoldaty.py intent. But the original shell script works fine also.

comment:176 Changed 7 years ago by gutow

OK guys. Either:

1) approve what is available now and works, even if it is not exactly the way you would like to see it done, and start a new ticket to get things done all inside python.

2) insist that things must be done all inside python. If that is the case, I cannot tell you how long it will take to get things working. Assuming things work as they are supposed to it will take at least until Monday or Tuesday for me to have enough time to properly test everything before putting up patches. If I re-encounter the kinds of problems I had with the subprocess routines before it might take even longer.

comment:177 Changed 7 years ago by strogdon

I vote for 1). Let's get this done now and address other things on another ticket.

comment:178 in reply to: ↑ 172 ; follow-up: Changed 7 years ago by ppurka

Replying to ddrake:

Replying to ppurka:

The #!/usr/bin/env bash syntax is recommended for portability.

As for the bash script, are you looking at the output of the bash script or only at the return status.

There's a doctest in sage/interfaces/jmoldata.py (line 90) that looks like this:

result = subprocess.call([testjavapath],stdout=jout)

Then my version will always have a return code of 0. This is why I asked the question. So we have the following choices:

  1. For python based solution: the python method in comment:173 looks good.
  1. For bash based solution: the simple bash version in the case you use subprocess.call is below. This will have the proper return code of 0 or 1, depending on whether java is present and is version 1.[5-7] or otherwise.
    #!/usr/bin/env bash
    type -atp java  && java -version 2>&1 | grep version.*[1]\.[567]
    exit $?
    
  1. A slightly more verbose bash version (again assuming you are using subprocess.call)
    #!/usr/bin/env bash
    type -atp java || exit 1  # exit if java is not found
    java -version 2>&1 | grep version.*[1]\.[567]
    exit $?
    
  1. Even more verbose bash script (assuming usage of subprocess.call) - the one in comment:166

The bash script currently shipped in the spkg does not return the correct exit codes.

comment:179 in reply to: ↑ 178 Changed 7 years ago by gutow

Replying to ppurka:

The bash script currently shipped in the spkg does not return the correct exit codes.

Yes it does. It does not send them to the standard output, but it does return the correct values to the $? variable that subprocess.call gets the result value from. Otherwise we would never get the error message on machines that don't have java or the correct version and we do. If you want to check that the script works simply do this on a machine with java 1.5-7

 >./testjava.sh
>echo $?
0

On a machine with a different java or no java you will see

>./testjava.sh
>echo $?
1

If you don't have a machine without java you can check it by changing the grep statement to

grep version.*[1]\.[2]

Then run the script again. You will find an exit code of 1.

I verified this on Ubuntu and MacOS.

Last edited 7 years ago by gutow (previous) (diff)

comment:180 Changed 7 years ago by ppurka

Well, the behavior I am getting on Linux is different (and what I expected). Let me first ensure that we are talking about the same script - jmol-12.3.27.p1/patches/testjava.sh (removing redundant lines):

#!/bin/bash
type -atp java
OUT=$?
if [ $OUT -eq 0 ]; then
  java -version 2>&1|grep version.*[1]\.[567]
fi

The problem is that if the type statement exits with a nonzero status, then OUT will be nonzero. The if statement will not execute and hence it will never reach the grep statement so as to give a nonzero return status. This can be checked by changing the line type -atp java with type -atp not_present_command. I get the following behavior

...z/jmol-12.3.27.p1/patches» bash -x ./testjava.sh
+ type -atp not_present_command
+ OUT=1
+ '[' 1 -eq 0 ']'
...z/jmol-12.3.27.p1/patches» echo $?
0

# With the original script and java present
...z/jmol-12.3.27.p1/patches» bash -x ./testjava.sh
+ type -atp java
/usr/bin/java
+ OUT=0
+ '[' 0 -eq 0 ']'
+ java -version
+ grep 'version.*[1].[567]'
java version "1.6.0_33"
...z/jmol-12.3.27.p1/patches» echo $?
0

comment:181 follow-up: Changed 7 years ago by ppurka

I think I need to clarify my statement too - the currently packaged script in the spkg will give the incorrect exit code only if java is not present. If java is present then it will behave properly.

EDIT: changed last sentence :S

Last edited 7 years ago by ppurka (previous) (diff)

comment:182 in reply to: ↑ 181 Changed 7 years ago by gutow

Yep! I see it. Simple fix. I will roll a new spkg and call it .p2, just to avoid confusion. I forgot that the if statement becomes the last command...my bad...

adding

else
  exit 1

to the if statement.

Replying to ppurka:

I think I need to clarify my statement too - the currently packaged script in the spkg will give the incorrect exit code only if java is not present. If java is present then it will behave properly.

EDIT: changed last sentence :S

comment:183 Changed 7 years ago by gutow

  • Description modified (diff)

switched to jmol-12.3.27.p2.spkg: now both checking for a JVM and JVM version checking should work.

comment:184 follow-up: Changed 7 years ago by kini

By the way, Jonathan, in case you weren't aware, you don't need to change the .p number in your SPKG version unless the previous .p version was actually published on the Sage mirrors.

comment:185 in reply to: ↑ 184 Changed 7 years ago by gutow

Thanks for the info. In this case it is just a good way to keep track of changes for the people doing review. Replying to kini:

By the way, Jonathan, in case you weren't aware, you don't need to change the .p number in your SPKG version unless the previous .p version was actually published on the Sage mirrors.

comment:186 in reply to: ↑ 158 ; follow-up: Changed 7 years ago by ddrake

With the new "p2" spkg, on a machine with no Java, I get a doctest failure in jmoldata.py exactly like that in comment 158. The new testjava.sh script works properly, though, so if the doctest failure is the intended behavior, I think we can set this to positive review.

(It does seem strange that on some machines, proper behavior will be to have failing doctests. Some people will surely be confused by that. But this upgrade is perhaps important enough to justify that.)

comment:187 in reply to: ↑ 186 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Replying to ddrake:

It does seem strange that on some machines, proper behavior will be to have failing doctests. Some people will surely be confused by that.

Indeed. A doctest failure should never be intentional. And I don't think we should require a recent version of Java in Sage just for this.

comment:188 Changed 7 years ago by jdemeyer

The correct thing to do is probably to mark the test

# optional - java

Changed 7 years ago by gutow

make java dependent doctests in jmoldata.py optional

comment:189 Changed 7 years ago by gutow

  • Description modified (diff)

comment:190 follow-up: Changed 7 years ago by gutow

So I've fixed:

  1. make java dependent doctests optional
  2. testjava.sh now properly checks for java and its version
  3. SPKG.txt fixes.

Is there anything else?

comment:191 in reply to: ↑ 190 Changed 7 years ago by ddrake

  • Reviewers changed from Karl-Dieter Crisman, Steven Trogdon, Punarbasu Purkayastha, John Palmieri to Karl-Dieter Crisman, Steven Trogdon, Punarbasu Purkayastha, John Palmieri, Dan Drake
  • Status changed from needs_work to positive_review

Replying to gutow:

So I've fixed:

  1. make java dependent doctests optional
  2. testjava.sh now properly checks for java and its version
  3. SPKG.txt fixes.

Those three things are fixed.

Is there anything else?

Gosh, I hope not. :) Looking through the comments, I think this is ready to go.

comment:192 follow-up: Changed 7 years ago by kcrisman

Thanks, Dan.

Gosh, I hope not. :) Looking through the comments, I think this is ready to go.

Until the inevitable complaints come about having to waken applets :) Jonathan, I assume that would be pretty hard to make a globally customizable thing, right?

comment:193 in reply to: ↑ 192 Changed 7 years ago by ppurka

Replying to kcrisman:

Thanks, Dan.

Gosh, I hope not. :) Looking through the comments, I think this is ready to go.

Until the inevitable complaints come about having to waken applets :) Jonathan, I assume that would be pretty hard to make a globally customizable thing, right?

I think heavy users of 3d plots will appreciate the speedups they get while opening worksheets ;)

comment:194 Changed 7 years ago by gutow

  • Milestone changed from sage-pending to sage-5.3

comment:195 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.3 to sage-5.4

comment:196 Changed 7 years ago by jdemeyer

Does jmol really depend on sagenb? It seems to me that the spkg-install script of jmol simply copies stuff to $SAGE_LOCAL, so I don't see the dependency.

comment:197 follow-up: Changed 7 years ago by gutow

The dependency is indirect. The functionality available from this package is accessed by the notebook. Also the javascript library in this package is specifically modified to work with the notebook jmol_lib.js. I tried not to break non-notebook operation of Jmol so in theory this should operate with sage even without the notebook installed. However, I did not test it without the notebook installed and do not know of anybody who has. The new notebook definitely depends on this package.

comment:198 in reply to: ↑ 197 ; follow-up: Changed 7 years ago by jdemeyer

Replying to gutow:

The dependency is indirect. The functionality available from this package is accessed by the notebook. Also the javascript library in this package is specifically modified to work with the notebook jmol_lib.js. I tried not to break non-notebook operation of Jmol so in theory this should operate with sage even without the notebook installed. However, I did not test it without the notebook installed and do not know of anybody who has. The new notebook definitely depends on this package.

Sorry, I should have been more clear, I meant build-time dependency, see the file spkg/standard/deps.

comment:199 in reply to: ↑ 198 Changed 7 years ago by gutow

Replying to jdemeyer:

Sorry, I should have been more clear, I meant build-time dependency, see the file spkg/standard/deps.

I cannot remember any and see nothing I am calling on in the .spkg.

comment:200 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.4.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.