openmmtools
openmmtools copied to clipboard
Avoid raising broad `Exception` when raising errors
Some check and errors are raised with generic Exception
objects, this can lead to some troubles in the future even if we are safe now.
We should enhance our code base and create and raise specific exceptions when dealing with errors.
As a first pass I detected the following parts in the code (not tests) where we may be facing this issue:
./openmmtools/alchemy.py: raise Exception("Not implemented; needs CustomMultipleForce")
./openmmtools/multistate/multistatesampler.py: raise Exception('Thermodynamic states contain a mixture of '
./openmmtools/multistate/multistatesampler.py: raise Exception('All sampler states must have box_vectors defined if the system is periodic.')
./openmmtools/multistate/multistatesampler.py: raise Exception('Cannot use MBAR with non-global locality.')
./openmmtools/multistate/sams.py: raise Exception('locality must be >= 1')
./openmmtools/multistate/sams.py: raise Exception('Initial logZ_guess (dim {}) must have same number of states as n_states ({})'.format(
./openmmtools/multistate/sams.py: raise Exception('Programming error: Unreachable code')
./openmmtools/multistate/sams.py: raise Exception('stage {} unknown'.format(self._stage))
./openmmtools/multistate/sams.py: raise Exception('Programming error: Unreachable code')
./openmmtools/multistate/multistateanalyzer.py: raise Exception('Non-global MBAR analysis not implemented yet.')
./openmmtools/multistate/multistateanalyzer.py: raise Exception("Cannot specify statistical_inefficiency without n_equilibration_iterations, because " \
./openmmtools/integrators.py: raise Exception('This integrator is not a CustomIntegrator.')
./openmmtools/integrators.py: raise Exception("Invalid Yoshida-Suzuki value. Allowed values are: %s"%
./openmmtools/integrators.py: raise Exception("Nosé-Hoover chain length must be at least 0")
./openmmtools/integrators.py: raise Exception("This integrator must be constructed with 'measure_shadow_work=True' to measure shadow work.")
./openmmtools/integrators.py: raise Exception("This integrator must be constructed with 'measure_heat=True' in order to measure heat.")
./openmmtools/integrators.py: raise Exception("This integrator must be Metropolized to return an acceptance rate.")
./openmmtools/integrators.py: raise Exception('nsteps_neq must be an integer >= 0')
./openmmtools/integrators.py: raise Exception("alpha must be in the interval (0,1); specified alpha = %f" % alpha)
./openmmtools/utils.py: raise Exception('Argument is not a type')
./openmmtools/utils.py: raise Exception(f"Platform {platform.getName()} unknown")
I think that unless we have some clear-cut ones that could use further classification, making these OpenmmtoolsError
or OpenMMToolsError
is probably good enough.
Also see #523
I was bit by this again recently and came here to make the issue :)
Good catch! Pretty much all of these should be ValueError
.