-
Notifications
You must be signed in to change notification settings - Fork 4
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
enhancement: energy_loss update #11
base: develop
Are you sure you want to change the base?
Conversation
Resolves #10 |
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.
This is an excellent start @dmulash, thanks for updating all the impacted methods!
I left a lot of comments, but they're mostly relating to the use of an electrical losses method, rather than an input. I have a couple of questions in there as well that'd be great to get your input on, especially while we're modifying the code. I also made a handful of smaller changes myself just to reduce some of the back and forth.
Hi @RHammond2, Apart from your suggested changes (eliminate Feel free to adjust as needed! |
@@ -1424,14 +1445,12 @@ def energy_production( | |||
by: str = "windfarm", | |||
units: str = "gw", | |||
per_capacity: str | None = None, | |||
with_losses: bool = False, | |||
loss_ratio: float | None = None, | |||
environmental_loss_ratio: float | None = None, |
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.
It looks like the energy losses weren't applied in this method, which should be done just before the AEP conditional.
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.
Basically the section you removed should be replaced with something like:
losses = self.energy_losses(...) # pass the parameters that were used for the energy potential basis
production = potential - losses
@@ -1624,6 +1631,228 @@ def energy_losses( | |||
|
|||
return losses / self.capacity(per_capacity) | |||
|
|||
def wake_losses( |
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 realized there was an error in how this was done, which was mostly my fault, so I fixed that. I would double check the remaining losses calculations, and replicate the process in this method to ensure that both the control flow and the calculations are correct.
@@ -1592,19 +1610,25 @@ def energy_losses( | |||
by="turbine", | |||
units="kw", | |||
) | |||
|
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.
This section should be roughly in the form of:
if not losses_breakdown:
production = ... # the code that's there already
losses = potentail.copy()
losses["total_losses"] = potential.values - production.values # can use a different column name
else:
# raise an error for if by == "turbine" because detailed breakdowns are only for the wind farm
environmental_lr = self.environmental_loss_ratio()
availability_lr = self.availability(which="energy", by="windfarm", frequency="month-year")
... # the remaining loss ratios
losses = potential.copy
losses = losses.assign(
environmental_losses=environmental_lr,
availability_losses=availability_lr,
... # the remaining lossses
).rename(columns={}) # final step if you think we should have them displayed more nicely
# convert to desired units
# aggregate to apporpriate time scale ("annual", "project")
# apply aep
# apply per_capacity
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.
From our conversation: only calculate the loss ratio, not the actual losses. (removed from the example above)
|
||
return total_loss_ratio | ||
|
||
def losses_breakdown( |
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.
This should really be incorporated into the energy_losses
method using a new keyword argument called breakdown
because the loss amounts may need to be broken down across time as well.
if self.floris_results_type == "wind_rose": | ||
power = pd.DataFrame(0.0, dtype=float, index=availability.index, columns=["drop"]) | ||
power = power.merge( | ||
energy_gwh = pd.DataFrame(0.0, dtype=float, index=availability.index, columns=["drop"]) |
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.
@RHammond2 fix this to get potential as the base energy, so losses can be subtracted.
…loss_ratio and descriptive note for total_loss_ratio method
No description provided.