metamolecular / inchi-js

InChI for the Browser
http://metamolecular.com/inchi-js
Other
14 stars 5 forks source link

flat model crashes inchi.js #1

Open BobHanson opened 8 years ago

BobHanson commented 8 years ago

Entry of the (albeit inappropriately) flat 2D taxol structure found at NCI/CADD http://cactus.nci.nih.gov/chemical/structure/taxol/file?format=sdf&get3d=True crashes or in some way permanently disables inchi.js. This is observable at http://chemapps.stolaf.edu/jmol/jsmol/inchi.htm if you put "taxol" into the search box and then press [Search]. An inchi is created:

InChI=1S/C47H51NO14/c1-25-31(60-43(56)36(52)35(28-16-10-7-11-17-28)48-41(54)29-18-12-8-13-19-29)23-47(57)40(61-42(55)30-20-14-9-15-21-30)38-45(6,32(51)22-33-46(38,24-58-33)62-27(3)50)39(53)37(59-26(2)49)34(25)44(47,4)5/h7-21,31-33,35-38,40,51-52,57H,22-24H2,1-6H3,(H,48,54) Omitted undefined stereo

but then if any additional model -- morphine, for example -- is loaded, the error

TypeError: a is undefined

is thrown from then on.

rapodaca commented 8 years ago

Using the online InChI.js test here:

http://metamolecular.com/inchi-js/

I'm not able to reproduce the problem you see. Does the same test work at the above link?

BobHanson commented 8 years ago

You can't reproduce it at my site? It does not have the problem at your site. For whatever reason.

On Wed, Mar 9, 2016 at 12:17 PM, Richard Apodaca notifications@github.com wrote:

Using the online InChI.js test here:

http://metamolecular.com/inchi-js/

I'm not able to reproduce the problem you see. Does the same test work at the above link?

— Reply to this email directly or view it on GitHub https://github.com/metamolecular/inchi-js/issues/1#issuecomment-194433605 .

Robert M. Hanson Larson-Anderson Professor of Chemistry Chair, Department of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr

If nature does not answer first what we want, it is better to take what answer we get.

-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900

rapodaca commented 8 years ago

I did try on the site I linked, but not your site. If you're not seeing the error on:

http://metamolecular.com/inchi-js/

then chances are the issue lies on your site. One way to debug this would be to start with the test page, which is included in the inchi.js repository. Then add the JS dependencies you're using on your site, one by one (just add the script tags, don't activate the libraries). If that doesn't trigger the error, then the problem could be elsewhere.

BobHanson commented 8 years ago

Yes, OK. I have tracked one problem down to Jmol, in that I see that java2script hijacks Error(). Correcting that, I see now:

uncaught exception: abort() at Clazz.declareTypeError/f@j2s /core/corejmol.z.js:1993:12 Ja@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:57:21 Ia@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:55:22 C@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:455:101 d.T._abort @file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:105:195 Kn@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:288:56470 Mn@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:288:69727 xk@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:110:16885 wk@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:110:13358 tg@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:287:30399 Uf@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:195:19770 $f@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:287:2973 _f@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:225:3 Sb@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:420:7 Eb@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:308:8 cb@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js:298:3 @file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi/inchi.js line 39 > eval:1:130 dotest@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi.htm:29:16 getInChI/<@file:///C:/jmol-dev/workspace/JSmol/site/jsmol/inchi.htm:13:26

On Wed, Mar 9, 2016 at 1:10 PM, Richard Apodaca notifications@github.com wrote:

I did try on the site I linked, but not your site. If you're not seeing the error on:

http://metamolecular.com/inchi-js/

then chances are the issue lies on your site. One way to debug this would be to start with the test page, which is included in the inchi.js repository. Then add the JS dependencies you're using on your site, one by one (just add the script tags, don't activate the libraries). If that doesn't trigger the error, then the problem could be elsewhere.

— Reply to this email directly or view it on GitHub https://github.com/metamolecular/inchi-js/issues/1#issuecomment-194456897 .

Robert M. Hanson Larson-Anderson Professor of Chemistry Chair, Department of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr

If nature does not answer first what we want, it is better to take what answer we get.

-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900

BobHanson commented 8 years ago

Rich,

OK, here you go:

http://chemapps.stolaf.edu/jmol/jsmol/i.htm

and, yes, it also behaves this way at your site. Just introduce the taxol data from NCI/CADD three or four times, and it will cause an abort() in your code. It is in the method "Kn()"

It is reproducible in Firefox, Chrome, and Edge.

​Bob

rapodaca commented 8 years ago

That's odd - I can't seem to reproduce the error. Browsing to:

http://metamolecular.com/inchi-js/

I click the document button and paste the molfile from:

http://cactus.nci.nih.gov/chemical/structure/taxol/file?format=sdf&get3d=True

I repeated 10x in a row without error. Running Chrome 48.0.2564.116 on OSX. Also tried on Firefox 43.0.4 OSX without a problem.

If I get a chance, I can try this with your exact configuration. What version of Windows and browsers were you using?

BobHanson commented 8 years ago

I just tried http://chemapps.stolaf.edu/jmol/jsmol/i.htm on my Mac using Safari -- same issue exactly. And my collaborator Otis Rotherberger has reported the same thing.

It looks to me to be some sort of memory caching issue.

Note that i.htm is a very clean file. Just:

<!DOCTYPE html>

JSmol-InChI test Bob On Wed, Mar 9, 2016 at 6:21 PM, Richard Apodaca notifications@github.com wrote: > That's odd - I can't seem to reproduce the error. Browsing to: > > http://metamolecular.com/inchi-js/ > > I click the document button and past the molfile from: > > http://cactus.nci.nih.gov/chemical/structure/taxol/file?format=sdf&get3d=True > > I repeated 10x in a row without error. Running Chrome 48.0.2564.116 on > OSX. Also tried on Firefox 43.0.4 OSX without a problem. > > If I get a chance, I can try this with your exact configuration. What > version of Windows and browsers were you using? > > — > Reply to this email directly or view it on GitHub > https://github.com/metamolecular/inchi-js/issues/1#issuecomment-194585582 > . ## Robert M. Hanson Larson-Anderson Professor of Chemistry Chair, Department of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr If nature does not answer first what we want, it is better to take what answer we get. -- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900
rapodaca commented 8 years ago

I just tried http://chemapps.stolaf.edu/jmol/jsmol/i.htm on my Mac using Safari -- same issue exactly.

Just to be clear...

When you try the test I outlined in my previous message (using the page http://metamolecular.com/inchi-js/), what do you see?

I ask because it wasn't clear whether you're reporting results for your page or the one I linked to.

I looked at your HTML file and can see the issue you refer to. However, there are a few oddities, including the way you store/load the molfile via DOM element. Here's a cleaner test:

https://gist.github.com/rapodaca/4070c58b466ce285a200

Copy this file into a directory, then replace the parts on lines 19 and 23 with links to the inchi.js.mem and inchi.js file. You should be able to click the button repeatedly without a problem.

Comparison of the gist I posted with the HTML you're using will probably provide clues to what is triggering the problem.

BobHanson commented 8 years ago

OK, that's it. So, for whatever reason, inchi.c crashes some odd time after given a 2D file that has no stereochemical descriptors. That's the difference between those two files. Turns out my file was not just from NCI. It was one created by Jmol from the NCI model, and because it was a 2D file to start with, it did not preserve the 1,6 bond stereochemistry descriptors. Sorry, I forgot I had done that. I guess I cannot hold that against it. Garbage in - garbage out. Nothing more than that. Although, good programming would say the program should not die because of that, perhaps.

As for the storage of data in a DOM element. Actually, that's a very clean way to do this - direct transfer of the file, none of this \n business. Very easy to parse. See, for example, http://chemapps.stolaf.edu/jmol/jsmol/lite3.htm. You should try it!

Thanks for your help. This inchi.htm page and the associated .js and .js.mem files will be distributed with JSmol now.

BTW, looking through that algorithm -- heck of a 4 MB integer array there! Must be the program heap.

Bob

rapodaca commented 8 years ago

OK, that's it. So, for whatever reason, inchi.c crashes some odd time after given a 2D file that has no stereochemical descriptors.

I don't follow. So far I haven't been able to reproduce your bug on a clean HTML file.

Did you run the demo I posted? If so, with which file and what was the outcome?

Actually, that's a very clean way to do this - direct transfer of the file, none of this \n business.

If you're not in favor of JavaScript variables a cleaner way is to put the data into a script tag:

When used to include data blocks (as opposed to scripts), the data must be embedded inline, the format of the data must be given using the type attribute, the src attribute must not be specified, and the contents of the script element must conform to the requirements defined for the format used.

https://www.w3.org/TR/html5/scripting-1.html#the-script-element

Alternatively, you can load the file with an XMLHttpRequest object.

It's still not clear to me that the method in which you stored data in your demo wasn't the cause of the problem.

BobHanson commented 8 years ago

On Thu, Mar 10, 2016 at 8:54 AM, Richard Apodaca notifications@github.com wrote:

OK, that's it. So, for whatever reason, inchi.c crashes some odd time after given a 2D file that has no stereochemical descriptors.

I don't follow. So far I haven't been able to reproduce your bug on a clean HTML file.

Sorry - Everything is fine. What I am saying is that it is no concern to you. My bad. I was using a 2D MOL file with no stereochemical descriptors, and that is garbage. I was using that; you were not. So that is why the problem occurred.

Did you run the demo I posted? If so, with which file and what was the outcome?

Actually, that's a very clean way to do this - direct transfer of the file, none of this \n business.

If you're not in favor of JavaScript variables a cleaner way is to put the data into a script tag:

When used to include data blocks (as opposed to scripts), the data must be embedded inline, the format of the data must be given using the type attribute, the src attribute must not be specified, and the contents of the script element must conform to the requirements defined for the format used.

https://www.w3.org/TR/html5/scripting-1.html#the-script-element

Alternatively, you can load the file with an XMLHttpRequest object.

I use all these methods all sorts of times. This was just so I could quickly put this together as simply as possible.

It's still not clear to me that the method in which you stored data in your demo wasn't the cause of the problem.

I can verify that it was not -- I have used this for years, and also when I popped in the file from NCI, it works perfectly. Seriously, it works fantastically. It is easy for a server to insert a file between two tags in the HTML comment, and then just this tiny bit of JavaScript to pull it out.

Bob

rapodaca commented 8 years ago

My bad. I was using a 2D MOL file with no stereochemical descriptors, and that is garbage.

Which molfile was that? I consider any indeterminate state to be a bug, regardless of whether the input is legal or not. I'd like to try the "2D MOL file with no stereochemical descriptors" you found in the test harness I linked to, to see that an error actually occurs, and hopefully, fix it.

BobHanson commented 8 years ago

That's the one that is embedded in the HTML file: http://chemapps.stolaf.edu/jmol/jsmol/i.htm

On Thu, Mar 10, 2016 at 9:23 AM, Richard Apodaca notifications@github.com wrote:

My bad. I was using a 2D MOL file with no stereochemical descriptors, and that is garbage.

Which molfile was that? I consider any indeterminate state to be a bug, regardless of whether the input is legal or not. I'd like to try the "2D MOL file with no stereochemical descriptors" you found in the test harness I linked to, to see that an error actually occurs, and hopefully, fix it.

— Reply to this email directly or view it on GitHub https://github.com/metamolecular/inchi-js/issues/1#issuecomment-194902794 .

Robert M. Hanson Larson-Anderson Professor of Chemistry Chair, Department of Chemistry St. Olaf College Northfield, MN http://www.stolaf.edu/people/hansonr

If nature does not answer first what we want, it is better to take what answer we get.

-- Josiah Willard Gibbs, Lecture XXX, Monday, February 5, 1900