microsoft / molskill

Extracting medicinal chemistry intuition via preference machine learning
MIT License
103 stars 9 forks source link

Getting segfault's from workers, but not sure what's causing it #6

Open rajarshi opened 1 year ago

rajarshi commented 1 year ago

Hi - thanks for the nicely packaged code. I've been running it on some internal data so can't share the inputs data. The code I'm running is

from molskill.scorer import MolSkillScorer
import numpy as np

d= [x.strip() for x in open("d.smi").read().split("\n")]

scorer = MolSkillScorer()
scores = scorer.score(d)
np.savetxt("d-molskill.csv", scores, delimiter=",")

However I see the follow errors - and as far as I can tell the input SMILES are all valid (they've been cleaned). Do you know what might cause this error?

Traceback (most recent call last):
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/ML/Descriptors/MoleculeDescriptors.py", line 88, in CalcDescriptors
    res[i] = fn(mol)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/Chem/EState/EState.py", line 76, in MaxEStateIndex
    return max(EStateIndices(mol, force))
ValueError: max() arg is an empty sequence
Traceback (most recent call last):
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/ML/Descriptors/MoleculeDescriptors.py", line 88, in CalcDescriptors
    res[i] = fn(mol)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/Chem/EState/EState.py", line 83, in MinEStateIndex
    return min(EStateIndices(mol, force))
ValueError: min() arg is an empty sequence
Traceback (most recent call last):
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/ML/Descriptors/MoleculeDescriptors.py", line 88, in CalcDescriptors
    res[i] = fn(mol)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/Chem/EState/EState.py", line 90, in MaxAbsEStateIndex
    return max(abs(x) for x in EStateIndices(mol, force))
ValueError: max() arg is an empty sequence
Traceback (most recent call last):
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/ML/Descriptors/MoleculeDescriptors.py", line 88, in CalcDescriptors
    res[i] = fn(mol)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/rdkit/Chem/EState/EState.py", line 97, in MinAbsEStateIndex
    return min(abs(x) for x in EStateIndices(mol, force))
ValueError: min() arg is an empty sequence
ERROR: Unexpected segmentation fault encountered in worker.
@Traceback (most recent call last):
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/torch/utils/data/dataloader.py", line 1120, in _try_get_data
    data = self._data_queue.get(timeout=timeout)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/multiprocessing/queues.py", line 113, in get
    if not self._poll(timeout):
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/multiprocessing/connection.py", line 257, in poll
    return self._poll(timeout)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/multiprocessing/connection.py", line 424, in _poll
    r = wait([self], timeout)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/multiprocessing/connection.py", line 931, in wait
    ready = selector.select(timeout)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/selectors.py", line 416, in select
    fd_event_list = self._selector.poll(timeout)
  File "/cluster/home/guha/miniconda3/envs/molskill/lib/python3.9/site-packages/torch/utils/data/_utils/signal_handling.py", line 66, in handler
    _error_if_any_worker_fails()
RuntimeError: DataLoader worker (pid 171390) is killed by signal: Segmentation fault. 
josejimenezluna commented 1 year ago

Hi @rajarshi,

Many thanks for testing our package out. This error seems to be related to a few RDKit descriptors not being able to be computed correctly - namely those from this module.

https://www.rdkit.org/docs/source/rdkit.Chem.EState.EState.html

I think there is little we can do here apart from removing these descriptors and retrain the default model - hoping that performance remains similar. Alternatively, could you please check for how many molecules in your set you cannot compute these descriptors and remove them before you pass them to the scorer?

EDIT: We'd like to catch these cases in the scorer nonetheless, so that we can return a nan value instead of crashing. Could you provide a example structure where this happens?

rajarshi commented 1 year ago

Could the code be updated to catch descriptor failures and skip that molecule (maybe logging the failures)

Unfortunately, I can't share the structures (also, it's not clear which structure is failing as that is not being logged)

josejimenezluna commented 1 year ago

Without having an example molecule where this happens, this is a bit hard to debug. We could try and catch the exception when computing the descriptors, but if you get a segfault this won't solve your issue.

Could you check whether you can avoid the segfault by setting num_workers=0 in the scorer?

greglandrum commented 1 year ago

One reasonable solution to failing descriptor calculations (this can always happen) is to have the training/application code catch the exceptions and insert a default value for each descriptor that fails. I normally use something that's clearly out of range and easy to recognize. @rajarshi this unfortunately doesn't help you, but you could add a pre-filter that catches molecules which throw exceptions during descriptor generation and prevents them from being passed to the model.