highspy: unpythonic error handling
The following python program exits with status code 0.
import highspy
h = highspy.Highs()
h.readModel("does_not_exist.mps")
h.run()
and this output:
Running HiGHS 1.7.2 (git hash: 44dd10c75-dirty): Copyright (c) 2024 HiGHS under MIT licence terms
ERROR: File does_not_exist.mps not found
Coefficient ranges:
Model status : Empty
HiGHS run time : 0.00
Clearly, exit code 0 is wrong as this was not a successful run. What should have been done instead? For a simple script, this would work:
import highspy
h = highspy.Highs()
assert h.readModel("does_not_exist.mps") == highspy.HighsStatus.kOk
h.run()
But if this code is part of a larger program that shouldn't crash right away, using an assert is not an option. If, for example, one wants proper exceptions to be handled by the caller, one could start guessing what the problem might have been and raise an appropriate exception:
import highspy
h = highspy.Highs()
if h.readModel("does_not_exist.mps") != highspy.HighsStatus.kOk:
raise FileNotFoundError("does_not_exist.mps")
h.run()
But then one has to add this boilerplate code around every highspy call and has to think about what the proper exception should be.
This problem is not limited to readModel, but (almost?) all highspy functions. For example, I couldn't even guess why addVar could not return Ok. I guess that would be some exceptional circumstance, which means it would be ok to raise an exception.
All in all, the current error handling in highspy feels like using a C API and not a python API. What's the solution? One way would be to define a HighsException and write python wrapper functions around every highs call, perform the error handling in the wrapper, and raise HighsException if necessary. But since there are people around here who are more knowledgable than me in writing python APIs, they might come up with better ideas.
EDIT: addVariable is already a wrapper that works like this and raises a generic Exception. There is a handful of other functions that work like this.
For
import highspy h = highspy.Highs() h.readModel("does_not_exist.mps") h.run()
My view is that, since h.readModel returns HighsStatus.kError, this should be checked by the user. But maybe this is what you mean when you say that "highspy feels like using a C API and not a python API"
After being passed a file name that does not exist, the incumbent model in HiGHS remains empty. Hence no error occurs when h.run() is executed, so the return of HighsStatus.kOk is appropriate.
Note
- There are very few HiGHS methods where internal errors can occur.
h.run()is the main case. - Almost all situations where HiGHS returns
HighsStatus.kErrorare due to errors in user data - such as the file name that does not exist. - All HiGHS methods that don't return data - where errors cannot occur - return a HighsStatus, even if it's vanishingly unlikely that an internal error could occur. In your example of
addVar, it's possible for a memory allocation error to occur - not that HiGHS traps such an occurrence!
If Python users debug by observing exceptions rather than checking return codes, then there's a discussion to be had. If HiGHS raised an exception, I think that C++/C programmers would be unhappy. Of course, I've admitted that HiGHS generally doesn't trap memory allocation errors, but these are very unlikely since problems that don't fit into memory are generally far too big to solve. We have seen this once, and added memory allocation checking to presolve. We should add it to the model "addition" methods.
For
import highspy h = highspy.Highs() h.readModel("does_not_exist.mps") h.run()
My view is that, since
h.readModelreturnsHighsStatus.kError, this should be checked by the user. But maybe this is what you mean when you say that "highspy feels like using a C API and not a python API"
Exactly. I'd say h.readModel("does_not_exist.mps") is very similar to this code:
with open("does_not_exist.txt") as f:
print(f.read())
which raises FileNotFoundError: [Errno 2] No such file or directory: 'does_not_exist.txt'. The current behaviour is very surprising for a python API. For C/C++-APIs people are used to APIs returning error codes as C has no exceptions and in C++ they are avoided often enough.
If HiGHS raised an exception, I think that C++/C programmers would be unhappy.
I don't argue for the C++ code of highs to throw exceptions. It can be done solely in the python wrapper as already happens for some functions like addVariable. It should throw a more specific exception (at least a generic HighsException) to distinguish different sources of exceptions though.
Thanks. Useful dialogue. Team HiGHS can do little in Python, so it's really up to the community to propose a better way of handling exceptions