Adding user-configurable slack type #239
Conversation
|
#238 should be merged first for its bug fixes. |
| for i, val in _md_violation_penalties.items(): | ||
| _branch_penalties = {} | ||
| _branches_with_slack = [] | ||
| for bn, branch in branches.items(): |
There was a problem hiding this comment.
I think we should have some sort of options or configuration object to control whether or nor things like slacks get added rather than inferring it from what is in the data dict. If someone wants to solve with slacks and then without, they have to modify the data dict. Perhaps more importantly, it looks like slacks will get added even if slack_type == SlackType.NONE.
There was a problem hiding this comment.
The existing behavior (in main) allows the user to add slacks to any transmission line by setting the violation_penalty to something other than None.
Perhaps slack_type == SlackType.NONE should override the violation_penalties set for the individual branches? And we leave them alone if the slack_type is anything else?
There was a problem hiding this comment.
I guess it should also be noted that in the case that somebody wants to solve with slacks and without: provided they don't set individual violation_penalty attributes on a branch, no update to the data dictionary is required. They can optionally set the transmission_flow_violation_cost system attribute to set the system-wide transmission violation cost.
There was a problem hiding this comment.
I think I would be fine with that. We could also raise an error if slack_type == SlackType.NONE and violation_penalties are specified.
There was a problem hiding this comment.
I opted to ignore the violation_penalty and issue a warning. That seems to be most common for data incongruities in Egret, probably because its my preference, and I think Egret chooses reasonable defaults which saves the user time if they don't otherwise need to modify their inputs.
There was a problem hiding this comment.
Sounds good. Thanks.
| @@ -37,7 +38,7 @@ | |||
| ] | |||
| ) | |||
|
|
|||
| def generate_model( model_data, uc_formulation, relax_binaries=False, ptdf_options=None, PTDF_matrix_dict=None ): | |||
| def generate_model( model_data, uc_formulation, relax_binaries=False, ptdf_options=None, PTDF_matrix_dict=None, slack_type=SlackType.BUS_BALANCE): | |||
There was a problem hiding this comment.
Shouldn't the default slack type be SlackType.NONE?
There was a problem hiding this comment.
To maintain the existing behavior of Egret, this is the correct default. I can make the case for all three:
SlackType.BUS_BALANCEis the existing unit commitment modelSlackType.TRANSMISSION_LIMITSis probably most common in industry practiceSlackType.NONEis probably most common in academic models.
| @@ -84,6 +89,7 @@ def _generate_model( model_data, | |||
| _relax_binaries = False, | |||
| _ptdf_options = None, | |||
| _PTDF_matrix_dict = None, | |||
| _slack_type = SlackType.TRANSMISSION_LIMITS, | |||
There was a problem hiding this comment.
This should match above, I agree.
|
Approved, pending passing tests. |
No description provided.