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

Segmentation fault with CoordinateSet in Python #62

Open
RMeli opened this issue Jun 22, 2021 · 7 comments
Open

Segmentation fault with CoordinateSet in Python #62

RMeli opened this issue Jun 22, 2021 · 7 comments

Comments

@RMeli
Copy link
Member

RMeli commented Jun 22, 2021

When trying to explicitly build a CoordinateSet object in Python as follows

import molgrid
from openbabel import pybel

obmol = next(pybel.readfile("sdf", "benzene.sdf"))
obmol.addh()

cs = molgrid.CoordinateSet(obmol.OBMol)

I get a segmentation fault:

[1]    13710 segmentation fault (core dumped)  python mg.py

I also tried to pass obmol directly (PyBel molecule) and to pass a molgrid.GninaIndexTyper object as second argument, but always obtained the same error (or just a MemoryError).


OS: Ubuntu 18.04
conda environment:

name: test
channels:
  - conda-forge
  - pytorch
  - open3d-admin
dependencies:
  - python
  - ipython
  - pip
  - numpy
  - scipy
  - openbabel
  - open3d
  - pytorch
  - pip:
    - molgrid
@RMeli
Copy link
Member Author

RMeli commented Jun 22, 2021

This problem seems to occur only with the pip-installed library. If I compile libmolgrid from source (within a Singularity container), everything seems to work as expected.

@dkoes
Copy link
Contributor

dkoes commented Jun 25, 2021

The most likely problem is your version of openbabel is different than the one included with the pip package. Since the OBMol objects aren't compatible, it fails miserably. There a number of ways this could be dealt with.

  1. Change the install procedure to build from src and/or require a specific version of openbabel. This would be the conda approach and I'm not sure is possible with pip
  2. Implement version insensitive wrappers when converting between openbabel python objects and openbabel C++ objects (e.g., write out the molecule as a string in python and then read it back in using C++ to get a new object instead of just copying the pointer)
  3. Minimally, check python version versus c++ version and output a warning telling the user they will need to install a certain version of openbabel for things to work.
  4. Do 3, but if the check fails do 2.
  5. Bundle a copy of python openbabel with molgrid to use (e.g. you would import molgrid.openbabel.pybel to get a guaranteed compatible version)

I'm going to settle for 3 for now. It is possible that there is some other problem besides the versions (e.g. how the library was built vs changes in the source code).

@dkoes
Copy link
Contributor

dkoes commented Jun 26, 2021

Can you provide the details of your system openbabel (or whatever openbabel package molgrid would be using)?

@RMeli
Copy link
Member Author

RMeli commented Jun 26, 2021

Thank you for looking into this. I think having a warning after spotting this incompatibility would be great!

I installed openbabel within the conda environment from the conda-forge channel. The version being installed was 3.1.1. The conda environment above was enough for me to reproduce the problem.

@dkoes
Copy link
Contributor

dkoes commented Sep 13, 2021

I can think of two solutions:
(1) Embed a version of openbabel within molgrid that is guaranteed to be compatible (but may not be compatible with system openbabel, so less than ideal for integrating with pre-existing code).
(2) Configure a conda package that builds molgrid (rather than getting it from pip) against the conda version of openbabel. Managing these sorts of dependencies is kinda of the point of using conda.

(1) is easy to do. (2) will have to wait until I have some poor sod to fob it off onto.

@RMeli
Copy link
Member Author

RMeli commented Sep 14, 2021

Given that (1) is less than ideal, I'd say (2) is the preferred option.

If you have any pointers I could have a look into it? I started looking at conda-forge, but it was unclear to me how to deal with CUDA packages.

@dkoes
Copy link
Contributor

dkoes commented Sep 14, 2021

Not really, having never done it. It looks like you need to come up with a build recipe. Figuring out the dependencies is probably the main difficulty and ensuring that you are building against an appropriate version of openbabel. I would look at the recipes for other packages. I would think making torch a dependency would pull in all the CUDA stuff you need.

Incidentally, v0.5.1, which is now in pypi, has openbabel as a submodule.

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

2 participants