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

Add Protocol Errors #441

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
8 changes: 8 additions & 0 deletions gufe/protocols/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
"""Defining processes for performing estimates of free energy differences"""

from .errors import (
MissingProtocolUnitError,
ProtocolAnalysisError,
ProtocolExecutionError,
ProtocolSetupError,
ProtocolUnitFailureError,
ProtocolValidationError,
)
from .protocol import Protocol, ProtocolResult
from .protocoldag import ProtocolDAG, ProtocolDAGResult, execute_DAG
from .protocolunit import Context, ProtocolUnit, ProtocolUnitFailure, ProtocolUnitResult
28 changes: 28 additions & 0 deletions gufe/protocols/errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# This code is part of OpenFE and is licensed under the MIT license.
# For details, see https://github.com/OpenFreeEnergy/gufe


# Protocol Errors
class ProtocolValidationError(Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be subclassing out of a "BaseGufeException" type? I'm thinking it might be useful to have a catch-all that isn't a Exception in some cases.

For example - it might be good for execute_DAG to capture all of these and prevent trying to restart the DAG (i.e. if these are known deterministic failures, then we should assume they will happen over and over again).

"""Error when the protocol setup or settings can not be validated."""


class ProtocolSetupError(Exception):
"""Error when executing the setup unit of the protocol."""


class ProtocolExecutionError(Exception):
"""Error when executing the production unit of the protocol."""


class ProtocolAnalysisError(Exception):
"""Error when trying to perform some analyses after the protocol has been executed."""


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May make sense to make a ProtocolDAGResultError(ProtocolError) class, and make the exceptions below subclass it?

# Protocol Results Errors
class MissingProtocolUnitError(Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean a missing UnitResult?

"""Error when a ProtocolDAGResult is massing a protocol unit."""
jthorton marked this conversation as resolved.
Show resolved Hide resolved


class ProtocolUnitFailureError(Exception):
"""Error when a ProtocolDAGResult contains an failed protocol unit."""
19 changes: 11 additions & 8 deletions gufe/protocols/protocoldag.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import networkx as nx

from ..tokenization import GufeKey, GufeTokenizable
from .errors import MissingProtocolUnitError, ProtocolUnitFailureError
from .protocolunit import Context, ProtocolUnit, ProtocolUnitFailure, ProtocolUnitResult


Expand Down Expand Up @@ -211,22 +212,24 @@ def unit_to_result(self, protocol_unit: ProtocolUnit) -> ProtocolUnitResult:

Raises
------
KeyError
if either there are no results, or only failures
MissingProtocolUnitError:
if there are no results for that protocol unit
ProtocolUnitFailureError:
if all units failed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if all units failed
if there are only failures for that protocol unit

"""
try:
units = self._unit_result_mapping[protocol_unit]
except KeyError:
raise KeyError("No such `protocol_unit` present")
raise MissingProtocolUnitError(f"No such `protocol_unit`:{protocol_unit} present")
else:
for u in units:
if u.ok():
return u
else:
raise KeyError("No success for `protocol_unit` found")
raise ProtocolUnitFailureError(f"No success for `protocol_unit`:{protocol_unit} found")

def unit_to_all_results(self, protocol_unit: ProtocolUnit) -> list[ProtocolUnitResult]:
"""Return all results (sucess and failure) for a given Unit.
"""Return all results (success and failure) for a given Unit.

Returns
-------
Expand All @@ -235,19 +238,19 @@ def unit_to_all_results(self, protocol_unit: ProtocolUnit) -> list[ProtocolUnitR

Raises
------
KeyError
MissingProtocolUnitError
if no results present for a given unit
"""
try:
return self._unit_result_mapping[protocol_unit]
except KeyError:
raise KeyError("No such `protocol_unit` present")
raise MissingProtocolUnitError(f"No such `protocol_unit`:{protocol_unit} present")

def result_to_unit(self, protocol_unit_result: ProtocolUnitResult) -> ProtocolUnit:
try:
return self._result_unit_mapping[protocol_unit_result]
except KeyError:
raise KeyError("No such `protocol_unit_result` present")
raise MissingProtocolUnitError(f"No such `protocol_unit_result`:{protocol_unit_result} present")

def ok(self) -> bool:
# ensure that for every protocol unit, there is an OK result object
Expand Down
10 changes: 6 additions & 4 deletions gufe/tests/test_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
ProtocolUnit,
ProtocolUnitFailure,
ProtocolUnitResult,
MissingProtocolUnitError,
ProtocolUnitFailureError
)
from gufe.protocols.protocoldag import execute_DAG

Expand Down Expand Up @@ -692,7 +694,7 @@ def test_missing_result(self, units, successes, failures):

assert not dagresult.ok()

with pytest.raises(KeyError, match="No success for `protocol_unit` found") as e:
with pytest.raises(ProtocolUnitFailureError, match="No success for `protocol_unit`:NoDepUnit\(None\) found"):
dagresult.unit_to_result(units[2])

def test_plenty_of_fails(self, units, successes, failures):
Expand Down Expand Up @@ -721,11 +723,11 @@ def test_foreign_objects(self, units, successes):
transformation_key=None,
)

with pytest.raises(KeyError, match="No such `protocol_unit` present"):
with pytest.raises(MissingProtocolUnitError, match="No such `protocol_unit`:NoDepUnit\(None\) present"):
dagresult.unit_to_result(units[2])
with pytest.raises(KeyError, match="No such `protocol_unit` present"):
with pytest.raises(MissingProtocolUnitError, match="No such `protocol_unit`:NoDepUnit\(None\) present"):
dagresult.unit_to_all_results(units[2])
with pytest.raises(KeyError, match="No such `protocol_unit_result` present"):
with pytest.raises(MissingProtocolUnitError, match="No such `protocol_unit_result`:ProtocolUnitResult\(None\) present"):
dagresult.result_to_unit(successes[2])


Expand Down
Loading