Support validate / filter for IndexedSet components using the index

[jsiirola]

Fixes #2655; supersedes #3304

Summary/Motivation:

IndexedSet components supported the validate and filter arguments, but those callbacks didn't provide the index. This prevented index-specific behavior, which is inconsistent to other indexed components. This PR builds on (and supersedes) #3304 to provide the index to the callbacks for both validate and filter. This implementation extends the standard Initializer() logic to support parameterized callbacks.

More history

We have historically supported filters on multidimensional scalar sets that look like this:

model.I = RangeSet(5)
def lower_triangular(m, i, j):
    return i<= j
model.II = Set(initialize=model.I*model.I, filter=lower_triangular)

This starts to break down if the Set itself is indexed:

model.I = RangeSet(5)
def lower_triangular(m, i, j, k):
    return i<= j and i <= k and j <= k
model.II = Set(model.I, initialize=model.I*model.I, filter=lower_triangular)

And can become ambiguous if the IndexedSet index or the individual SetData objects are jagged sets.

To remove the ambiguity, this PR moves to a new paradigm where the "extra data" is not expanded when passed to the function:

model.I = RangeSet(5)
def lower_triangular(m, val, k):
    i, j = val
    return i<= j and i <= k and j <= k
model.II = Set(model.I, initialize=model.I*model.I, filter=lower_triangular)

Changes proposed in this PR:

• Add "Parameterized" initializer classes to the Initializer system
• Convert IndexedSet to using Initializers to process the validate and filter arguments
• Deprecate support of "expanded" value arguments to the validate and filter arguments when used on IndexedSet objects (TBD: discussion as to if we should also deprecate that API for scalar Set objects)

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.

[codecov]

Codecov Report

Attention: Patch coverage is 87.17949% with 15 lines in your changes missing coverage. Please review.

Project coverage is 88.53%. Comparing base (404fd6d) to head (c5a4fa4).

Files	Patch %	Lines
pyomo/core/base/initializer.py	71.79%	11 Missing ⚠️
pyomo/core/base/set.py	94.59%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3338   +/-   ##
=======================================
  Coverage   88.53%   88.53%           
=======================================
  Files         868      868           
  Lines       98495    98550   +55     
=======================================
+ Hits        87199    87248   +49     
- Misses      11296    11302    +6     

Flag	Coverage Δ
linux	86.05% <87.17%> (-0.01%)	⬇️
osx	75.63% <87.17%> (-0.01%)	⬇️
other	86.55% <87.17%> (+<0.01%)	⬆️
win	83.86% <87.17%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jsiirola]

Note that this code (silently) preserves the old API for scalar Set objects. However, that old API is

• difficult to reconcile with the "newer" API for IndexedSets
• widely used in Pyomo models in the wild

I think I'd like to put in a deprecation warning here, but that would generate a LOT of noise (basically, it would generate deprecation warnings for every model that uses filter= or validate=).

Thoughts (@blnicho, @mrmundt, @emma58, @michaelbynum, @whart222)?

[mrmundt]

I don't mind disturbing users in the wild, but I want the deprecation warnings to be helpful / tell them how to change over their models. What is the new, preferred method for doing this? Either we detail that directly in the deprecation message or we make a new page on RTD that gives more detailed instructions.

[jsiirola]

We had a long discussion at the Dev call today and decided that we should start issuing deprecation warnings for ScalarSets now.

[blnicho]

Should the order of idx and value be switched?

[blnicho]

Same question here about the ordering of idx and value. The deprecation warning matches the argument ordering I was expecting but the call to fcn does not. Is there magic happening in the initializer function call logic to swap argument ordering?

[blnicho]

I think I'm answering my own question by noticing that the argument ordering is indeed swapped here.

[blnicho]

@jsiirola there isn't really anything for you to resolve in my comments/questions, I just wanted to document my initial confusion. I am curious why the argument ordering and subsequent reordering was necessary. I'm guessing it had to do with having more control over the format of the index being passed in.

[jsiirola]

Yeah - this turned into a bit of a mess (and yes, it is confusing that they were reversed). The calls you are seeing here are all internal to the Initializer methods, so the user is never aware of them. In this case, because the "Parameterized" initializer was new, the old fcn API was just (block, index). Parameterized was adding "value", so it made sense for the extra arg to come at the end. This was further confused by the historical behavior where we would expand the value arg and pass it to the user-provided callback (leading things like the messy deprecation paths around set.py lines 1482-1520), and leaving things like this seemed to require less code gymnastics to implement the deprecation paths and make things compatible with the rest of the Initializer system.