gnina / libmolgrid

Comprehensive library for fast, GPU accelerated molecular gridding for deep learning workflows
https://gnina.github.io/libmolgrid/
Apache License 2.0
144 stars 48 forks source link

Segmentation fault with CoordinateSet in Python #62

Open RMeli opened 3 years ago

RMeli commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

RMeli commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.