-
Notifications
You must be signed in to change notification settings - Fork 518
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
Support validate / filter for IndexedSet components using the index #3338
Merged
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
c109901
Using the validate argument with indexed set objects now allows acces…
a06e997
Added more tests to ensure the functionality is maintained.
47d63a3
Merge branch 'Pyomo:main' into valsetindex
pmlpm1986 e85a339
Applied black.
ad01644
Revised tests.
b232a11
Applied black.
0cf9c7c
Applied black.
bc54e01
Revised test statements.
080cddd
Applied black again.
57203c6
Nothing.
c502ef9
Merge branch 'valsetindex' into valsetindexprior
a239857
Merge pull request #4 from pmlpm1986/valsetindexprior
pmlpm1986 cf7af78
Applied black yet again.
5aa048b
Merge branch 'main' into valsetindex
pmlpm1986 a94b7a4
Removed unnecessary pass statement.
8795578
Merge branch 'main' into valsetindex
pmlpm1986 257d24f
reverting original change to Set validation
jsiirola e7a6ee5
Merge branch 'main' into valsetindex
jsiirola c780c22
Merge branch 'valsetindex' of https://github.com/pmlpm1986/pyomo into…
jsiirola 12227d0
Move InitializerBase to leverage AutoSlots
jsiirola 52be7a7
Support parameterized initializer functions (that take additional arg…
jsiirola 8f6b9ba
Revert changes to old tests/examples
jsiirola f2a67ee
Switch Set to use Initializer to process validate callback
jsiirola 9402a8e
Expand tutorial, update tests
jsiirola fc1d8ee
Move Set filter callback to leverage Initializer / validate handler l…
jsiirola 7bb1f2f
Remove _validate and _filter from SetData (and only store on container)
jsiirola aa3ea48
NF: apply black
jsiirola b24f0a4
Update baseline with additional Set example
jsiirola 4d825f6
Merge branch 'main' into valsetindex
jsiirola 9506873
Provide implicit filter callback argument mapping for scalar Sets
jsiirola f61a98b
Fix typo, variable naming convention
jsiirola c5a4fa4
Merge branch 'main' into valsetindex
jsiirola 46ef920
Resolve inconsistency in ParameterizedInitializer call API
jsiirola 757baba
Resolve bug when wrapping new function types with additional_args
jsiirola 6a581b1
Improve Initializer / Set unit test coverage
jsiirola df7f2b2
Clarify deprecation message
jsiirola 2d47311
Deprecation warning for scalar sets with filter/validate callbacks ex…
jsiirola e64143d
Update pyomo/core/base/initializer.py
blnicho File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm answering my own question by noticing that the argument ordering is indeed swapped here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 aroundset.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.