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

Update wind rose and power rose #392

Merged
merged 27 commits into from
Sep 8, 2022
Merged

Conversation

paulf81
Copy link
Collaborator

@paulf81 paulf81 commented Mar 15, 2022

Feature or improvement description
Would like to update and reincorporate the wind rose and power rose tools. There are some points I have open questions:

  • @ejsimley : is wind toolkit stuff still current / need an update?
  • @Bartdoekemeijer : do you have preferences on putting things in current/matching style of wd/ws/turbine matrices?

@paulf81 paulf81 added enhancement An improvement of an existing feature v3 Label to denote focus on v3 labels Mar 15, 2022
@paulf81 paulf81 added this to the v3.1.0 milestone Mar 15, 2022
@Bartdoekemeijer
Copy link
Collaborator

Hi Paul. I do not have a strong preference for putting things in a certain format. With this being separated from FlorisInterface, I'd say we are free to format things however we think makes most sense. Also, would you like me to look at anything of the classes specifically?

@bayc bayc modified the milestones: v3.1.0, v3.2.0 Apr 6, 2022
@paulf81
Copy link
Collaborator Author

paulf81 commented Apr 7, 2022

Maybe this one is getting closer but I had a couple of questions:

@ejsimley : at the bottom of the wind rose file there are a number of tools related to getting data from the wind toolkit, do you know, do these still work? Or has the recommended usage changed? Do we still want this all in, or does this now belong in flasc or pruf or something? What do you think?

@Bartdoekemeijer and @ejsimley : similar question, scrolling through power rose, I was wondering if this isn't doing things we handle more intrinsically in V3 since the results come back vectorized?

@paulf81
Copy link
Collaborator Author

paulf81 commented Apr 7, 2022

for my part, I've updated 07b and confirms it works with new no_wake style and also matches the results from example 07

Copy link
Collaborator

@Bartdoekemeijer Bartdoekemeijer left a comment

Choose a reason for hiding this comment

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

Just a quick couple comments! I agree that power_rose has been pretty much replaced by FLORIS v3. power_rose does have a couple functions like plotting by direction and additionally calculating a baseline scenario and a no_wake scenario, but that's very easy with FLORIS v3 at this point, too. I vote that we just remove power_rose, and perhaps only carry over the visualization function to floris.tools.visualization.

freq = wind_rose.df.set_index(['wd','ws']).unstack().values


# Verify dimensions of the variable "freq"
Copy link
Collaborator

Choose a reason for hiding this comment

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

After this point, could you just use fi.get_farm_AEP?

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 is a good suggestion @Bartdoekemeijer , just commited a version that does this

* (np.abs(sigma_n) ** (4 / n))
* (1 - sum_lbda) ** 2
)
tmp = a2 - (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this fix go in a separate Pull Request? Is this generic enough to be merged back into develop or is this just a temporary fix for something?

@paulf81
Copy link
Collaborator Author

paulf81 commented Jun 1, 2022

hi @rafmudaf, @Bartdoekemeijer and @bayc I think this pull request is basically ready but there's probably some sequencing to do:

1). For a project that required it, it contains pieces of @bayc 's layout optimization refactorization, if that pull is merged into develop, those apparent file changes should disappear
2) It also includes a fix to the cumulative curl model which in theory could go in separately, @rafmudaf what do you prefer?
3) Then it contains a fake 20MW turbine I needed in another project but which could be deleted at the last moment and I can re-add untracked

@rafmudaf
Copy link
Collaborator

rafmudaf commented Jun 1, 2022

  1. It also includes a fix to the cumulative curl model which in theory could go in separately, @rafmudaf what do you prefer?

I'd say the best is to include that in a separate pull request and make it to the main branch. However, if that's a pain, at least it would be good to describe that change in the original post of this pull request.

@paulf81
Copy link
Collaborator Author

paulf81 commented Jun 1, 2022

done! see pull request #438

@Bartdoekemeijer
Copy link
Collaborator

I think it looks good, but the additional commits surrounding layout optimization makes it hard to identify whether each change is really required. I think I'd like to have #429 merged before we merge this one. That'll make things clearer.

@paulf81
Copy link
Collaborator Author

paulf81 commented Jun 3, 2022

I think it looks good, but the additional commits surrounding layout optimization makes it hard to identify whether each change is really required. I think I'd like to have #429 merged before we merge this one. That'll make things clearer.

Agreed!!

@bayc bayc marked this pull request as ready for review September 8, 2022 20:17
@bayc bayc merged commit 647ed14 into NREL:develop Sep 8, 2022
@rafmudaf rafmudaf mentioned this pull request Sep 12, 2022
5 tasks
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 floris.tools v3 Label to denote focus on v3
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants