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

Save WindData onto FlorisModel and simplify post-run() calls #849

Merged
merged 27 commits into from
Mar 22, 2024

Conversation

misi9170
Copy link
Collaborator

@misi9170 misi9170 commented Mar 20, 2024

This pull request simplifies calls to methods that extract the power after a FlorisModel has been run(). To do so, any WindData object that is passed in to FlorisModel.set() is saved onto the FlorisModel. Setting of the FlorisModel._wind_data attribute can only be achieved using set(), and a getter method is added to expose the hidden FlorisModel._wind_data.

These changes enable the following calls to be simplified considerably, and allow the output to match the "shape" of the WindData object, if one was used:

  • get_turbine_powers()
  • get_farm_power()
  • get_farm_AEP()

A few important changes are also implemented:

  • get_farm_AEP() is now simply a wrapper around a new method, get_expected_farm_power(). The get_expected_farm_power() is then clearer. We have also discussed raising an error if get_farm_AEP() is called without a WindRose data type on FlorisModel._wind_data---@paulf81, what do you think, should get_farm_AEP() raise an error if called without a WindRose?
  • None of the get_...power methods call run(). To use these methods, the user must call run() first. This clarifies the paradigm, which used to differ between different methods (get_farm_AEP() would call run(), whereas get_turbine_powers() and get_farm_power() would not).
  • The above means that the no_wake flag can be removed form the the get_farm_AEP() method---if the user would like a version without wakes, they will now execute run_no_wake() instead of run() prior to calling the get_...power methods.

Other notes

To do

  • Update UncertainFlorisModel to track these changes.
  • Consider switching to warning, rather than error, if layout optimization is run without a WindData object available
  • Consider raising error or warning if get_farm_AEP() is called without a WindData object available

@misi9170 misi9170 added enhancement An improvement of an existing feature v4 Focus of FLORIS v4 labels Mar 20, 2024
@misi9170
Copy link
Collaborator Author

misi9170 commented Mar 20, 2024

@rafmudaf , before you review fully, are you on board with the main idea of this PR?
If so, @paulf81 , perhaps you could map these changes onto UncertainFlorisModel (which would be a good way of getting to know the changes I've made here...)

Also, @ejsimley , see the FlorisModel.wind_data attribute (which is actually a getter over a hidden attribute FlorisModel._wind_data) and the FlorisModel.get_expected_farm_power() method

@misi9170 misi9170 added this to the v4.0 milestone Mar 20, 2024
floris/floris_model.py Outdated Show resolved Hide resolved
@ejsimley
Copy link
Collaborator

Thanks @misi9170. I think this is a good way to simplify getting farm power, expected power, AEP and saving/accessing the wind rose information in the floris model.

@paulf81
Copy link
Collaborator

paulf81 commented Mar 20, 2024

hi @misi9170 , one thought is you might discard the changes to the examples because they will only make the eventual merge to refactor_examples harder, if you leave the core changes in place I can update all the relavent examples in refactor_examples

@misi9170
Copy link
Collaborator Author

hi @misi9170 , one thought is you might discard the changes to the examples because they will only make the eventual merge to refactor_examples harder, if you leave the core changes in place I can update all the relavent examples in refactor_examples

That's true... but the checks will fail and the examples on the v4 branch won't run in the meantime. Is that a problem?

@paulf81
Copy link
Collaborator

paulf81 commented Mar 20, 2024 via email

@misi9170
Copy link
Collaborator Author

misi9170 commented Mar 21, 2024

Hi @paulf81, to be honest, I'd rather keep them in to keep a record of the changes necessary for the examples to run that will be tied to this PR's merge commit. Assuming this PR gets merged, when you go to merge v4 back into the refactor_examples branch at some point, you could just keep all "current" changes (i.e. the versions on the refactor_example branch) to the example files (and then make updates to the examples as necessary). I don't think that should be too much of a headache, but I'm certainly happy to help deal with the merge conflict if it's messier than I'm imagining!

@paulf81
Copy link
Collaborator

paulf81 commented Mar 21, 2024

Hi @paulf81, to be honest, I'd rather keep them in to keep a record of the changes necessary for the examples to run that will be tied to this PR's merge commit. Assuming this PR gets merged, when you go to merge v4 back into the refactor_examples branch at some point, you could just keep all "current" changes (i.e. the versions on the refactor_example branch) to the example files (and then make updates to the examples as necessary). I don't think that should be too much of a headache, but I'm certainly happy to help deal with the merge conflict if it's messier than I'm imagining!

Ok, yeah, I get that. Let's do it that way then, thanks!

@misi9170
Copy link
Collaborator Author

For future reference, note a small new capability that tests if a warning is logged (e.g on this file)

@misi9170 misi9170 merged commit 724f452 into NREL:v4 Mar 22, 2024
8 checks passed
@misi9170 misi9170 deleted the v4-ms/wind_data-on-fmodel branch March 22, 2024 21:09
@misi9170
Copy link
Collaborator Author

@aclerc, just wanted to tag you here so that you see this, since this change is partly in response to your feedback :)

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 v4 Focus of FLORIS v4
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants