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

NL writer: Add custom exception for no free variables #3445

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion pyomo/repn/plugins/nl_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@
)


class NLWriterEmptyModelError(ValueError):
"""
A custom exception to allow handling of a
model with no free variables.
"""

pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion here: We have a "custom" exception from pyomo/common/errors called PyomoException that you might want to inherit from instead. We do this fairly frequently for custom error types so it is also clear when an exception comes from Pyomo as opposed to something else (e.g., gurobipy or idaes).

Example:

from pyomo.common.errors import PyomoException

class MyCustomError(PyomoException):
   """
   My custom error
   """

Also note that you don't need to put pass as long as you have a docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, I initially had it inheriting from ValueError since that was raised previously, but I don't suppose there is much need for backwards compatibility in this case!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind, there were a couple of tests designed to catch this - I have replaced the asserted exception in those tests



# TODO: make a proper base class
class NLWriterInfo(object):
"""Return type for NLWriter.write()
Expand Down Expand Up @@ -320,7 +329,7 @@ def __call__(self, model, filename, solver_capability, io_options):
if config.symbolic_solver_labels:
os.remove(row_fname)
os.remove(col_fname)
raise ValueError(
raise NLWriterEmptyModelError(
"No variables appear in the Pyomo model constraints or"
" objective. This is not supported by the NL file interface"
)
Expand Down
Loading