feat: allow inspection of variable definitions#103
Closed
KnorpelSenf wants to merge 2 commits intomainfrom
Closed
Conversation
Collaborator
|
I don't think mutating variables after their creation should be allowed. But you can add accessors to read the values |
Contributor
Author
|
Will do! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This makes the fields in variable definitions public. That solves two problems I have.
Cannot Inspect Variable Definitions
Before variable definitions are added to the problem variables, they're very simple containers. After creating many of them, I'd like to log detailed information about their definitions so that I know what I'm passing to the solver.
Currently, all fields are hidden. There is no way to inspect them after building them. Thus I'm currently forced to roll my own copy of variable definitions which let me do these inspections. I then translate them to the good_lp variable definitions (which effectively is a no-op). This could be improved by making the fields public.
Cannot Mutate by Reference
Another issue is that if one has an
&Vec<VariableDefinition>then it is currently impossible to add a name, bounds, an int constraint, or an initial value to any of the definitions because the builder pattern always requires ownership. We could clone a variable and then overwrite the old value, but it feels wrong to do several copies just to flip a boolean.This PR allows for a different workaround. The field can now be set directly by mutating the definition, which is better.