openforcefield / openff-toolkit

The Open Forcefield Toolkit provides implementations of the SMIRNOFF format, parameterization engine, and other tools. Documentation available at http://open-forcefield-toolkit.readthedocs.io
http://openforcefield.org
MIT License
318 stars 92 forks source link

Make stereochemistry warning a real warning #1045

Open lilyminium opened 3 years ago

lilyminium commented 3 years ago

Is your feature request related to a problem? Please describe.

The "warning" (not error because allow_undefined_stereo=True) is not actually a warning emitted by the warnings module, but instead logged at the "warning" level. It's much more difficult to filter logging messages from the OpenFF logger than to filter warnings. At times there are so many such warnings that, for example, it clogs up the Jupyter output cell and I'm unable to see what I'm actually trying to output because the maximum character limit has been reached.

Given that allow_undefined_stereo=False by default, the fact that the user has turned allow_undefined_stereo=True indicates that they are somewhat aware that they do not have sufficient stereochemistry in their SMILES or rdkit molecule. As such a user I find these "warning"s of negligible value but significant inconvenience when it drowns out other output.

Describe the solution you'd like

Make the "warning" logging message a real warning. That makes it far easier for users to filter warnings by messages.

Describe alternatives you've considered

Status quo

Additional context

Currently I set the following in all my scripts, which makes me worry that I am missing warnings of real value.

offlogger = logging.getLogger("openff")
offlogger.setLevel(logging.ERROR)
offlogger.propagate = False

I tried filtering the logger with the below code, but was not successful. I'm not sure why; I tried loggers from "openff", "openff.toolkit.topology", "openff.toolkit.utils", "openff.toolkit.topology.molecule", "openff.toolkit.utils.toolkits".

class IgnoreStereoFilter(logging.Filter):
    def filter(self, record):
        return "(not error because allow_undefined_stereo=True)" not in record.getMessage()
offlogger.addFilter(IgnoreStereoFilter())

from openff.toolkit.topology import Molecule
mol = Molecule.from_smiles("[H][N]([C]([H])([C](=[O])[*:2])[C]([H])([H])[H])[*:1]", allow_undefined_stereo=True)
Warning (not error because allow_undefined_stereo=True): OEMol has unspecified stereochemistry. oemol.GetTitle(): 
Problematic atoms are:
Atom atomic num: 6, name: , idx: 2, aromatic: False, chiral: True with bonds:
bond order: 1, chiral: False to atom atomic num: 7, name: , idx: 1, aromatic: False, chiral: False
bond order: 1, chiral: False to atom atomic num: 1, name: , idx: 3, aromatic: False, chiral: False
bond order: 1, chiral: False to atom atomic num: 6, name: , idx: 4, aromatic: False, chiral: False
bond order: 1, chiral: False to atom atomic num: 6, name: , idx: 7, aromatic: False, chiral: False

Related to #1044

mattwthompson commented 1 year ago

It'd be nice to be more consistent on warnings vs. logging - this is probably the single most impactful change we could make to improve this