Use specific exception classes where appropriate

[bdice]

Description

I tried to read a file that doesn't exist with sim.create_state_from_gsd. Should GSD file reading errors be OSError and/or FileNotFoundError instead of RuntimeError?

**ERROR**: dump.gsd: Bad file descriptor - lattice.gsd

---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-3-3623b04ad812> in <module>
     11 
     12 sim.operations.integrator = mc
---> 13 sim.create_state_from_gsd(filename='lattice.gsd')

~/miniconda3/envs/dice/lib/python3.8/site-packages/hoomd/simulation.py in create_state_from_gsd(self, filename, frame)
    107                                         self.device._cpp_exec_conf)
    108         # Grab snapshot and timestep
--> 109         reader = _hoomd.GSDReader(self.device._cpp_exec_conf,
    110                                   filename, abs(frame), frame < 0)
    111         snapshot = Snapshot._from_cpp_snapshot(reader.getSnapshot(),

RuntimeError: Error reading GSD file

Proposed solution

Use another exception instead of RuntimeError, perhaps FileNotFoundError or OSError.

Additional context

I think this may apply to other exceptions as well - we use RuntimeError pretty liberally in our code bases and it seems like it's a catch-all for things that have more specific causes. The docstring says

Raised when an error is detected that doesn’t fall in any of the other categories. The associated value is a string indicating what precisely went wrong.

I frequently refer to this list of all built-in exceptions when choosing what to raise. It may be useful to other developers. https://docs.python.org/3/library/exceptions.html#exception-hierarchy

[vyasr]

While I agree that HOOMD probably overuses RuntimeError, I think the underlying issue in many cases (including this example) is not in Python, but rather in C++ code that throws runtime_error, which gets transparently mapped to Python's RuntimeError. Fixing this more thoroughly will require using different C++ exceptions. Fortunately, in addition to mapping a number of built-in C++ exceptions to corresponding Python exceptions, pybind11 also defines additional exceptions to map to otherwise inaccessible Python exceptions from C++, and it even provides a mechanism for mapping custom C++ exceptions to Python ones. While the custom exception mapping is probably overkill for our purposes, making more judicious choices of exceptions in the C++ code (including some of the built-in pybind11 exceptions) would probably be quite helpful. A simple search for runtime_error in C++ headers and sources should identify the places to be checked.

[joaander]

pybind11 does not define any default mapping to OSError or FileNotFoundError: https://pybind11.readthedocs.io/en/stable/advanced/exceptions.html We will need to use custom exceptions to throw these.

[joaander]

There is another patten we should fix when we work on this issue. In most places, HOOMD issues an error and then raises an exception. Example:

        m_exec_conf->msg->error() << "Requesting type name for non-existent type " << type << endl;
        throw runtime_error("Error mapping type name");

Instead, this should not call the error and put the descriptive string in the exception. I've been using ostringstream for this when I update it, but it is cumbersome. Is there a better way? Example:

        std::ostringstream s;
        s << "No supported GPUs are present on this system." << std::endl;
        for (const auto& msg : s_gpu_scan_messages)
            s << msg << std::endl;
        throw runtime_error(s.str());

[bdice]

@joaander Changing that "log error and except" pattern would be a great idea. I think std::ostringstream is about as good as you can get. There are some other suggestions here but I support the approach you took in that snippet above.

[joaander]

I have made slow progress in converting the error, then throw calls to just a throw with the full error description as outlined above. There are still many places in the codebase to check and convert. We should make a concerted effort to convert them all over before releasing 3.0.0.