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

Issue alert if calc wake needs to be run #432

Merged
merged 9 commits into from
Jul 27, 2022

Conversation

paulf81
Copy link
Collaborator

@paulf81 paulf81 commented May 12, 2022

Just a first pass at an idea to address Issue #431

@Bartdoekemeijer what do you think?

Feature or improvement description
Check a certain attribute exists to determine if calc wake needs to be run

Related issue, if one exists
Issue #431

@paulf81 paulf81 added the enhancement An improvement of an existing feature label May 12, 2022
@paulf81 paulf81 added this to the v3.2.0 milestone May 12, 2022
@@ -0,0 +1,15 @@

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is nice to demonstrate, but we should remember to remove this example file before merging

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed

@@ -558,12 +558,20 @@ def check_wind_condition_for_viz(self, wd=None, ws=None):
if len(ws) > 1 or len(ws) < 1:
raise ValueError("Wind speed input must be of length 1 for visualization. Current length is {}.".format(len(ws)))

def check_calc_wake_run(self, func_name):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe make this private, _check_calc_wake_run?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion, changed it

@@ -558,12 +558,20 @@ def check_wind_condition_for_viz(self, wd=None, ws=None):
if len(ws) > 1 or len(ws) < 1:
raise ValueError("Wind speed input must be of length 1 for visualization. Current length is {}.".format(len(ws)))

def check_calc_wake_run(self, func_name):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe make this private, _check_calc_wake_run?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed

def get_turbine_powers(self) -> NDArrayFloat:
"""Calculates the power at each turbine in the windfarm.

Returns:
NDArrayFloat: [description]
"""

self.check_calc_wake_run('get_turbine_powers')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! We also need to add it to get_farm_power, and maybe some of the visualization functions?

def get_turbine_powers(self) -> NDArrayFloat:
"""Calculates the power at each turbine in the windfarm.

Returns:
NDArrayFloat: [description]
"""

self.check_calc_wake_run('get_turbine_powers')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! We also need to add it to get_farm_power, and maybe some of the visualization functions?

@paulf81 paulf81 changed the title first pass Issue alert if calc wake needs to be run May 24, 2022
@rafmudaf rafmudaf self-assigned this May 26, 2022
@rafmudaf rafmudaf marked this pull request as ready for review June 1, 2022 22:01
@rafmudaf
Copy link
Collaborator

rafmudaf commented Jun 1, 2022

I'm not totally convinced about this State pattern that I've added here. The idea is that we can control the "state" of any object directly without relying on a derived metric to know its state. @RHammond2 what do you think? Overkill or useful?

@RHammond2
Copy link
Collaborator

@rafmudaf, I'm not totally convinced on it, though not opposed either, but I've also never quite found Enums to be useful in my own use cases, so I could be biased.

@rafmudaf
Copy link
Collaborator

rafmudaf commented Jun 9, 2022

I've also never quite found Enums to be useful in my own use cases

I'm not tied to the Enum type either, but mostly wondering about the idea of having a State attribute

@paulf81 paulf81 merged commit 3cca335 into NREL:develop Jul 27, 2022
@paulf81 paulf81 deleted the feature/clear_error_message branch July 27, 2022 20:36
@rafmudaf rafmudaf mentioned this pull request Sep 12, 2022
5 tasks
@@ -631,6 +639,10 @@ def get_farm_power(
# for turbine in self.floris.farm.turbines:
# turbine.use_turbulence_correction = use_turbulence_correction

# Confirm calculate wake has been run
if self.floris.state is not State.USED:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@paulf81 I think we do not need this here since it would be caught by self.get_turbine_powers(). Do you agree?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that makes sense to me, maybe this could be deleted and then we can make a test we get an error if calc wake not run before get_turbine_powers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants