Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Core dump when calling MolFromInchi #1206

Open
mnowotka opened this issue Dec 15, 2016 · 16 comments
Open

Core dump when calling MolFromInchi #1206

mnowotka opened this issue Dec 15, 2016 · 16 comments

Comments

@mnowotka
Copy link
Contributor

Hi,

I'm using RDkit version 2016.09.2:

import rdkit
rdkit.__version__
'2016.09.2'

Compiled with InChI support. Now for this InChI string:

InChI=1S/C5H12N14O7/c6-1(16-18(23)24)8-10-3(20)12-14-5(22)15-13-4(21)11-9-2(7)17-19(25)26/h(H12-2,6,7,8,9,10,11,12,13,14,15,16,17,20,21,22,23,24,25,26)/p+2

RDKit crashes (Seg fault):

from rdkit import Chem
mol = Chem.MolFromInchi("InChI=1S/C5H12N14O7/c6-1(16-18(23)24)8-10-3(20)12-14-5(22)15-13-4(21)11-9-2(7)17-19(25)26/h(H12-2,6,7,8,9,10,11,12,13,14,15,16,17,20,21,22,23,24,25,26)/p+2")

Segmentation fault (core dumped)

The same thing happens using the indigo toolkit:

import indigo
import indigo_inchi
indigoObj = indigo.Indigo()
indigo_inchiObj = indigo_inchi.IndigoInchi(indigoObj)
mol = indigo_inchiObj.loadMolecule(‘InChI=1S/C5H12N14O7/c6-1(16-18(23)24)8-10-3(20)12-14-5(22)15-13-4(21)11-9-2(7)17-19(25)26/h(H12-2,6,7,8,9,10,11,12,13,14,15,16,17,20,21,22,23,24,25,26)/p+2')

Segmentation fault (core dumped)

So I suspect, that the crash may be inside InChI library. But is there any way to protect from this? With the Seg fault there is no possibility to catch anything and properly handle this error.

@mnowotka
Copy link
Contributor Author

On the other hand it's possible to convert this Inchi to InchiKey and google it. The only result gives this:
http://jglobal.jst.go.jp/public/20090422/201407086094865150

@greglandrum
Copy link
Member

I agree that the fact that Indigo also seg faults indicates that the problem is likely in the InChI library itself.

Given that the problem is likely in the InChI library, I don't think there's anything that the RDKit can really do about it. But I'll look into it.

If you haven't already: it would be worth reporting the crash to the InChI folks.

@mnowotka
Copy link
Contributor Author

I did via twitter as they don't have any other channel. I'm not a chemistry expert but would it be possible to predict this problem before feeding this to inchi lib?

@greglandrum
Copy link
Member

Ok, I found the crash and have a simple fix that requires a patch to the inchi code. Now it just fails instead of failing and generating a seg fault. I'll get that committed (though it will likely only work when InChI is downloaded by the RDKit build. I still need to figure that out).

I'll also see if I can figure out who to send the patch to.

@mnowotka
Copy link
Contributor Author

Great, I always try to use the download-inchi.sh script where I can (I can't use it everywhere because it uses wget, it would be sooo much better if it tried wget OR curl instead), so this will work for me.

Out of curiosity, can you paste a link to the commit?

@greglandrum
Copy link
Member

It will be linked here automatically after I make it

@mnowotka
Copy link
Contributor Author

Perfect, thanks!

@kiddr
Copy link

kiddr commented Dec 16, 2016

Hi - thanks for the twitter notification. If you could send the issue/patch to the inchi-discuss list on sourceforge that's probably best, or mail me at richard at inchi-trust org

@kiddr
Copy link

kiddr commented Jan 3, 2017

We think this is fixed in 1.05, publicly released shortly (final review on inchi-discuss to 10 Jan 2017 if you want to take a look)

@mnowotka
Copy link
Contributor Author

mnowotka commented Jan 3, 2017

Nice, thanks. Would it be possible to get a patch I could apply to earlier versions to prevent the crash?

@greglandrum
Copy link
Member

@kiddr : it looks like a fairly large number of changes have been made to the InChI distribution. Certainly the new zips are laid out differently than the old ones and no longer build with the RDKit.
This makes testing "challenging".
I'll subscribe to the mailing list to discuss this

@mnowotka
Copy link
Contributor Author

mnowotka commented Jan 3, 2017

@greglandrum , you said you have a simple fix for rdkit. Can you share it?

@greglandrum
Copy link
Member

Yeah, sorry I haven't done that yet. I was trying to get it "right" and then I got distracted by the holidays.
Here's the change I made to runinchi.c:

~/rdk/RDKit_git/External/INCHI-API/src.o % diff -w runichi.c runichi.c.orig 
1754c1754
<             if ( prb_file && prb_file->f && 0L <= sd->fPtrStart && sd->fPtrStart < sd->fPtrEnd && !ip->bSaveAllGoodStructsAsProblem ) {
---
>             if ( prb_file->f && 0L <= sd->fPtrStart && sd->fPtrStart < sd->fPtrEnd && !ip->bSaveAllGoodStructsAsProblem ) {
1759c1759
<             if ( sd->nErrorCode && prb_file && prb_file->f && 0L <= sd->fPtrStart && sd->fPtrStart < sd->fPtrEnd && !ip->bSaveAllGoodStructsAsProblem ) {
---
>             if ( sd->nErrorCode && prb_file->f && 0L <= sd->fPtrStart && sd->fPtrStart < sd->fPtrEnd && !ip->bSaveAllGoodStructsAsProblem ) {
4006a4007,4008
> 
> 

@mnowotka
Copy link
Contributor Author

mnowotka commented Jan 3, 2017

Great, thanks!

@greglandrum
Copy link
Member

Please let me know if that works for you.

@mnowotka
Copy link
Contributor Author

mnowotka commented Jan 3, 2017

Will do.

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

No branches or pull requests

3 participants