Update_W fails to update dual weights in presence of arbitrary nonanticipative variables

[brentertainer]

To start, I am running the latest main branch (presently 52cc831).

In Pyomo it is sometimes convenient to introduce arbitrary decision variables and just let the solver ignore them. I have done this in the attached example -- the variable sales[0] is arbitrary, and I introduced it solely to avoid needing special logic for setting up the root node in the scenario tree.

inventory.tar.gz

When I execute run.sh, I encounter an issue in the Update_W function of spbase.py:

https://github.com/Pyomo/mpi-sppy/blob/main/mpisppy/phbase.py#L224

The issue is that the arbitrary decision variable is not assigned a value by the solver, so the code attempts to subtract a float from None. It is evident to me that somebody else has already encountered this issue given the commented print lines preceding the subtraction.

Intuitively, if I force sales[0] == 0 (a constraint that is commented out in inventory.py), then there is no issue. Unintuitively, the issue also goes away if I change --bundles-per-rank=0 to --bundles-per-rank=1 or --bundles-per-rank=3.

It took me a while to figure out why mpi-sppy was complaining. I suppose it would be nice for such a condition to be automatically detected and handled. Alternatively, it would probably save someone some time down the road if the resulting TypeError were captured and given some additional messaging. Something like "Check that your instance does not have arbitrary decision variables."

[jeanpaulwatson]

In the context of PySP, we would refer to sales as a "derived" first stage variable - it's value is determined once other "real" first-stage variables are specified. The reason I bring this up is that we didn't post non-anticipativity constraints for derived first-stage variables in PySP. And the analog seems to be your case. I agree that the error message can be significantly improved, but is there any reason why you wouldn't just omit sales from the list of variables for the root node?

[bknueven]

Intuitively, if I force sales[0] == 0 (a constraint that is commented out in inventory.py), then there is no issue. Unintuitively, the issue also goes away if I change --bundles-per-rank=0 to --bundles-per-rank=1 or --bundles-per-rank=3.

Both of these actions add an additional constraint on that variable, which will cause the variable to be passed to the solver. (In the bundles case, a non-anticipative constraint is created by mpi-sppy.) If these variables are really required, a potential work-around would be to add a meaningless constraint for each variable, e.g., (-1e20, sales[0], 1e20), so it gets included.

In many cases, having such stale variables be non-anticipative could be indicative of a modeling error. I suppose mpi-sppy could check the stale flag before accessing value, but this would add quite a few checks in places where it otherwise wouldn't be required. Perhaps in mpi-sppy we could be checking the stale flag on all the variables we're tracking after the call to solve, to at least print a useful error message?

[jeanpaulwatson]

I do worry about the overhead on the stale flag, but I suppose we should try it first and see if there is a big run-time hit on large-ish problems. If so, then we could always add a "--safe" run option that does all kinds of extra checks. Which would not be a bad idea anyway.

[brentertainer]

I agree that the error message can be significantly improved, but is there any reason why you wouldn't just omit sales from the list of variables for the root node?

@jeanpaulwatson Not a good reason, only that it's another if-statement in the construction of ScenarioNode objects.

@bknueven Thanks, it did not occur to me at first that mpi-sppy would create a non-anticipative constraint when bundles per rank is non-zero, but it's clear now that you mention.

Just one more thought -- to me, it is less an issue that mpi-sppy behaves in this way and more an issue that someone might not be able to diagnose the problem quickly based on the currently reported symptom (TypeError). I like the idea to conditionally check each variable's stale flag based on some sort of --safe or --debug option.