Skip to content
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

Transmission flow #834

Merged
merged 20 commits into from
Mar 24, 2022
Merged

Conversation

Janie115
Copy link
Contributor

@Janie115 Janie115 commented Nov 3, 2021

No description provided.


m.tx_simple_min_transmit_power_mw = Param(
m.TX_SIMPLE_BLN_TYPE_HRZS_W_MIN_CONSTRAINT,
within=Reals, default=0
Copy link
Member

@anamileva anamileva Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Janie115, I think it would be better to default to -Infinity here, which would make it possible to skip the if logic in the min_transmit_power_rule() below. Simply set a variable called Infinity at the top of the module like this Infinity = float('inf'). There's an example in gridpath/system/markets/volume.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anamileva I made the modification.

anamileva
anamileva previously approved these changes Nov 22, 2021
@anamileva anamileva dismissed their stale review November 22, 2021 22:43

I'm realizing this may be better done through the Tx availability construct

**Enforced Over**: TX_SIMPLE_OPR_TMPS_W_MIN_CONSTRAINT

Transmitted power should exceed the defined minimum transmission power in
each operational timepoint.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Janie115, could you add some explanation of the logic in the docstring here. I'm still not sure what we are trying to do. Is tx_simple_min_transmit_power_mw the amount that must be transmitted in either direction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is the amount that must be transmitted in one direction or the other. If the min transmit power is positive, then the transmitted power must exceed this value in the positive direction. If the min transmit power is negative, then the transmitted power must exceed this value in the negative direction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, sorry for my confusion. I have a few thoughts I'll share in a bit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused earlier by the use of "min" in the parameter name and am realizing about defaulting to negative infinity probably didn't make much sense. I suggest the following: let's have two parameters here, tx_simple_min_flow_mw that defaults to Negative_Infinity and tx_simple_max_flow_mw that defaults to Infinity. We'll then have two constraints TxSimple_Transmit_Power_MW>= tx_simple_min_flow_mw and TxSimple_Transmit_Power_MW<= tx_simple_max_flow_mw. I think this is a more intuitive way to achieve the same effect without the if logic and it allows for extra flexibility with bounding on both sides and even forcing a particular level of flow. Thoughts?

@anamileva anamileva self-requested a review November 23, 2021 18:01
Copy link
Member

@anamileva anamileva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Janie115, now that I think I understand what we are trying to do better (sorry I've been slow), here are a few thoughts.

**Enforced Over**: TX_SIMPLE_OPR_TMPS_W_MIN_CONSTRAINT

Transmitted power should exceed the defined minimum transmission power in
each operational timepoint.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused earlier by the use of "min" in the parameter name and am realizing about defaulting to negative infinity probably didn't make much sense. I suggest the following: let's have two parameters here, tx_simple_min_flow_mw that defaults to Negative_Infinity and tx_simple_max_flow_mw that defaults to Infinity. We'll then have two constraints TxSimple_Transmit_Power_MW>= tx_simple_min_flow_mw and TxSimple_Transmit_Power_MW<= tx_simple_max_flow_mw. I think this is a more intuitive way to achieve the same effect without the if logic and it allows for extra flexibility with bounding on both sides and even forcing a particular level of flow. Thoughts?

if var == 0:
return Constraint.Skip
elif var > 0:
for tmp in mod.TMPS_BY_BLN_TYPE_HRZ[bt, h]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of iterating over the timepoints in a balancing_type-horizon in the constraint rule definition, I would suggest creating the equivalent set (project, bh, h, tmp) and enforcing the constraint over that set. At the end, this is a timepoint-level constraint and you want to be able to see what timepoints it binds in (right now, you would only see the balancing_type-horizon when the constraint binds, so you are losing resolution).

Related to this is whether to even define the min and max flow parameters over a balancing_type-horizon or if it's better to just define them by timepoint. I would recommend the latter as it gives us more flexibility. Defining a balancing_type-horizon level parameter makes sense if we would be aggregating over that balancing_type-horizon, which here we are not. There trade-offs of course: in one case we need to define the parameter over a smaller set (balancing_type-horizons instead of timepoints) but we do need to create that set; in the other case, it's more cumbersome to define the parameter over each timepoint (as it's potentially the same over all those timepoints) but we do not need to worry about defining the balancing_type-horizons in the temporal structure.

Copy link
Contributor Author

@Janie115 Janie115 Jan 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anamileva I just reopened this code to try to finish it.

Regarding your first comment on having two parameters tx_simple_min_flow_mw and tx_simple_max_flow_mw and two constraints, I think how you propose to define the two constraints can be confusing. For example, if we want to impose a minimum flow in the negative direction of the transmission line, we would need to put a negative value for the tx_simple_max_flow_mw parameter, so that TxSimple_Transmit_Power_MW<= tx_simple_max_flow_mw.

@anamileva anamileva self-requested a review March 14, 2022 17:16
@Janie115 Janie115 changed the title Tx min transmit power Transmission flow Mar 15, 2022
each operational timepoint.
"""
var = mod.tx_simple_min_flow_mw[l, tmp]
if var == 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be if var == Negative Inifinity (skip constraint with default values, i.e. when not specified)?

each operational timepoint.
"""
var = mod.tx_simple_max_flow_mw[l, tmp]
if var == 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be if var == Inifinity (skip constraint with default values, i.e. when not specified)?

anamileva
anamileva previously approved these changes Mar 22, 2022
@anamileva
Copy link
Member

@Janie115, could you please resolve the conflicts in scenarios.csv and then this will be ready to merge?

@anamileva anamileva merged commit daf2ce5 into blue-marble:develop Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants