NL writer: Add custom exception for no free variables

[alma-walmsley]

Fixes #3411.

Summary/Motivation:

nl_writer.py raises a ValueError for when no free variables are found when writing the nl file. However, this is potentially a valid case in my situation (ie. everything is defined; all expressions can just be evaluated). I don't want to try...catch for a ValueError because it is a pretty generic exception, and I do want to error if there are other problems.

See the linked issue for a full description.

Changes proposed in this PR:

• Add a custom exception NLWriterEmptyModelError, which inherits from ValueError. This means a try...catch around the solve statement (with this specific exception) can be used to handle this case gracefully.

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

1. I agree my contributions are submitted under the BSD license.
2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

[alma-walmsley]

Related to #3131 / #3135. However, that indicates that this error should only occur when the legacy __call__ method is used.

For context, I am running into this issue with:

solver = SolverFactory("ipopt")
solver.solve(m)

[alma-walmsley]

Some feedback may be useful as I'm not sure if there are any implications on presolved values or other things I'm unaware of. My use case is "everything is defined", so solving can be skipped.

[mrmundt]

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.

[alma-walmsley]

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!

[alma-walmsley]

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

[alma-walmsley]

pyomo/repn/plugins/ampl/ampl_.py has the same exception in the _print_model_NL function (line 1365) ... should I look at adding the custom exception there as well?

[jsiirola]

I have spent the last week debating (with myself [and losing]) what the "expected" behavior here should be.

The current implementation dates back to the original NL writer, and followed the logic that since some mix of AMPL / the solver will die a horrible death of you give it an NL file with no variables, then we should consider a model with no free variables an "error" (and raise an exception). That seemed reasonable, but now with the introduction of the presolver, it is possible that a "valid" model could be square and completely solvable in the presolve (leaving us with an empty NL file). In this case, we can't raise an exception because we need the NL writer to return the presolve information so that we (the solver / caller) can record / load the computed values into the variables. That means that we need to move the check for an "empty" NL file from the writer and into the solver, and raises the question as to if this exception should be removed entirely. The argument against that is that an "empty" model can be a sign of a modeling error (something like forgetting to correctly load data into indexing sets, so everything is empty), and the exception was meant to prevent the user from assuming that everything was fine (when in fact there was nothing to do).

I am wondering if the following is a reasonable path forward:

• this PR is a good (and necessary) change (we have long held that Pyomo needs to revisit the exceptions it raises and add specificity where it makes sense). That said:
  • I think this is a more general issue than just the NL writer, so I would propose renaming it EmptyModelError
  • it should be defined in pyomo/common/errors.py
  • it should inherit from both ValueError (for backwards compatibility) and PyomoException (for consistency with other Pyomo errors)
  • we should use this error in the NL writers (v1 and v2), and probably the LP, BAR, and GAMS writers as well
• we should deprecate the behavior of raising EmptyModelError exceptions in the writers. Since there is no way to change the behavior (to not raise an exception) and to preserve backwards compatibility, I propose logging a deprecation warning before raising the error and then adding functionality to pyomo/future.py to allow users to switch to the "new" behavior (where no error is raised).
  • it would probably be good for this to be both a global switch (for changing the behavior globally) and a context manager (so specific solver interfaces can move to the new model early).
  • it is an open question (at least for me) as to if the solvers should now raise the EmptyModelError exception for "truly empty" models, or if they should just log a warning.

Thoughts?

[alma-walmsley]

This sounds good to me. To summarise:

• Keep the empty model check for the old nl writer interface(s), but it should use an EmptyModelError
• The new nl writer interface leaves the check up to the solver interface that called it.
  • Don't raise an exception for empty models because of a successful presolve - maybe include a log message? In any case, the presolved values need to be loaded back into the model.
  • Either raise the EmptyModelException or log a warning for "truly empty" models.

[alma-walmsley]

If I understand correctly, the new solver interface already has support for skipping a solve if it gets presolved?

Something like this works:

from pyomo.environ import ConcreteModel, Var, Constraint
from pyomo.contrib.solver.factory import SolverFactory

m = ConcreteModel()
m.x = Var()
m.c = Constraint(expr=m.x == 1)

opt = SolverFactory("ipopt")
result = opt.solve(m, tee=True)

print(m.x.value)

but interestingly, this doesn't (is this any different from the "truly empty" case?)

from pyomo.environ import ConcreteModel, Var
from pyomo.contrib.solver.factory import SolverFactory

m = ConcreteModel()
m.x = Var()
m.x.fix(1)

opt = SolverFactory("ipopt")
result = opt.solve(m, tee=True)

print(m.x.value)

[alma-walmsley]

Also, I think it doesn't necessarily make sense to limit the scope of this PR to just fixing the exception raised by the old nl writer. It is probably a good idea to fix things properly (and on my end, we should look at moving to the new solver factory interface).

[mrmundt]

@alma-walmsley - preemptive warning that I have a PR that is going to be up sometime in the next few weeks that restructures the new solver interfaces a bit, so the import path won't be exactly the same (it'll most likely be something like pyomo.contrib.solver.common.factory).

[alma-walmsley]

I created the base exception EmptyModelError in pyomo.common.errors, and added this to the nl v1 and v2 writers.

---

It seems that the LP and BAR writers (not sure about GAMS) currently overcome the empty model problem by adding an "empty" variable/constraint if there are none in the model:

ONE_VAR_CONSTANT = Var(name='ONE_VAR_CONSTANT', bounds=(1, 1))

I think it might make sense to avoid calling the solver with a meaningless variable/constraint, and transition towards using the EmptyModelError exception / allow skipping the solve if this occurs.

---

I also had a look at the new ipopt solver interface in pyomo.contrib.solver.ipopt.

It currently has this check:

if (
    config.raise_exception_on_nonoptimal_result
    and results.solution_status != SolutionStatus.optimal
):
    raise RuntimeError(
        'Solver did not find the optimal solution. Set '
        'opt.config.raise_exception_on_nonoptimal_result = False to bypass this error.'
    )

It could probably be preceded here by something like this:

if results.termination_condition == TerminationCondition.emptyModel:
    if config.raise_exception_on_empty_model:  # this could default to true?
        raise EmptyModelError(
            "No variables appear in the model constraints or objectives."
            "Set opt.config.raise_exception_on_empty_model = False to bypass this error."
        )
    # else don't check for non-optimal result
    # and no need to "load solutions"

I realise now there is still a lot to do with the new solver interfaces, I am wondering what the plan is for adding them? Long term, does every supported solver get its own solver interface file? How might checks like these be handled (while avoiding code duplication)?

[jsiirola]

@alma-walmsley: sorry this PR is dragging on -- most of the dev team has been out for the last 3 weeks (between holidays and an IDAES team meeting), and we have been distracted resolving issues that cropped up in the CI/CD infrastructure.

I completely agree with your comment about getting this fixed "right" (and not necessarily just for the NL v1/v2 writers). I generally like the approach you have here, and we are likely to move forward with something very similar. We were discussing this at the weekly Dev call, and there were some proposals that built on the list of exceptions with some alternate names (including a base InvalidModelError exception with derived exceptions ZeroFreeVariables, ...). I am hoping that we can do a "solver mini-sprint" in the near future (maybe this month) to work through some of this and really make progress finalizing the new solver interface standard.

To answer a couple of your additional questions above:

• yes: we are moving to where each solver has it's own interface. In the original design of Pyomo, there was a hope that we would only have to maintain a couple "generic" interfaces. However, over time it is clear that every solver has some kind of "one-off" issue.
• The GAMS and BAR writers / interfaces are in need of a refactor (over the last ~2 years we have rewritten the LP and NL interfaces ... BAR and GAMS are next up), so I would not look to them for anything that should be copied / imitated. For example, the ONE_VAR_CONST actually comes from the LP writer (several solvers cannot handle LP files with constant terms in the objective - the ONE_VAR_CONST was added to the LP writer so that we could include the constant term. That way the objective reported by the solver log would match the objective the user wrote). It is not needed for (and actually in some cases is harmful to) GAMS and BARON. It is there because those interfaces were developed by copying the LP interface and mildly editing it.

Finally, reach out to @mrmundt or myself if you are interested in joining the Dev calls (Tuesdays @ 12:30 MT [currently UTC-7]) and we will send you the dial-in info.

[alma-walmsley]

Fantastic, sounds good to me. I suppose I might as well join a dev call, assuming it is via something like zoom...? How should I get in touch with you?

[jsiirola]

The easiest address is [email protected] (that should reach a good number of us).