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

Move FlorisInterface .reinitialize() / .calculate_wake() to .set() / .run() #823

Merged
merged 40 commits into from Feb 27, 2024

Conversation

rafmudaf
Copy link
Collaborator

@rafmudaf rafmudaf commented Feb 23, 2024

Adopt .set() / .run() flow for FlorisInterface workflows

Currently, the typical flow for using FlorisInterface is to build a workflow around the .reinitialize() and .calculate_wake() functions. #801 suggests to add an additional step between these two for setting turbine operating conditions. In the subsequent discussion (see #801 (reply in thread)), the idea to refactor this workflow resulted in the following suggestion:

fi = FlorisInterface.from_dict(...)
fi.set(
    wind_directions=...,
    wind_speeds=...,
    any_other_input=...
)
fi.run()

This pull request implements the suggested workflow by adding FlorisInterface.set() and FlorisInterface.run() and removing FlorisInterface.reinitialize() and FlorisInterface.calculate_wake().

The FlorisInterface.set() function allows for setting any of the supported inputs or operation settings individually or in batch:

# These two are equivalent
fi.set(
    wind_directions=...,
    wind_speeds=...,
    any_other_input=...
)

fi.set(wind_directions=...)
fi.set(wind_speeds=...)
fi.set(any_other_input=...)

For any parameter that is part of the input (defined in the input file or input dictionary to FlorisInterface and Floris), the .set() function creates a new Floris object with the setting changed. This matches the functionality of .reinitialize(). Therefore, changes should be batched into one .set() when possible to save the overhead of creating the new objects. In either case, the parameters that aren't provided as arguments to the function are not changed in Floris including the operation settings.

If either FlorisInterface.calculate_wake() or FlorisInterface.reinitialize() are used from v3, an error is raised pointing the user to the upgrade guide.

RIP calculate_wake() 💀

Related issue

Related to #801

Impacted areas of the software

FlorisInterface and any script or function that uses it.

@rafmudaf rafmudaf self-assigned this Feb 23, 2024
@rafmudaf rafmudaf added v4 Focus of FLORIS v4 enhancement An improvement of an existing feature floris.tools labels Feb 23, 2024
@rafmudaf rafmudaf modified the milestones: v3.6, v4.0 Feb 23, 2024
@rafmudaf
Copy link
Collaborator Author

Consider this flow for multiple calculations as implemented in #823:

f = Floris.from_dict(...)

# Get power with non-zero yaw
f.set(wind_directions=..., wind_speeds=..., yaw_angles=...)
f.run()
yawed_power = f.get_farm_AEP()

# Get power with no yaw
f.reset_operation()    # Sets all operating set-points to their default
f.run()
baseline_power = f.get_farm_AEP()

This start to look like Floris is retaining memory, and this is the same paradigm we intended to remove in the shift from v2 to v3. In fact, f.reset_operation() resets the operating set-points and creates a new Floris object, so the memory is not retained. However, the user-facing API still implies that we're operating on a single instance.

@bayc I'm wondering whether you read this the same way, and if you have any feedback on this given our experience with v2 and the shift to v3. For my part, this flow feels like a backward step, but I think I'm stuck too much on the previous API's to give this a fair evaluation.

ejsimley and others added 2 commits February 23, 2024 15:28
@paulf81
Copy link
Collaborator

paulf81 commented Feb 26, 2024

@rafmudaf I noticed a test failing and traced it to a small issue in test_disabled_turbines and pushed up a fix

@paulf81
Copy link
Collaborator

paulf81 commented Feb 26, 2024

These seems ready to merge from my point of view, only outstanding comment I see is a question on "TODO" item in the parellel_computing_interface, but that is really minor so I will approve

@paulf81 paulf81 self-requested a review February 26, 2024 16:36
@rafmudaf
Copy link
Collaborator Author

@misi9170 It would be good to get your eyes here again when you have a chance. I think I've addressed the tests that we had discussed last week.

@misi9170
Copy link
Collaborator

@misi9170 It would be good to get your eyes here again when you have a chance. I think I've addressed the tests that we had discussed last week.

Thanks for setting those tests up @rafmudaf --- they look good to me.

@misi9170
Copy link
Collaborator

These seems ready to merge from my point of view, only outstanding comment I see is a question on "TODO" item in the parellel_computing_interface, but that is really minor so I will approve

I've now cleaned that up

# Conflicts:
#	examples/34_wind_data.py
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 v4 Focus of FLORIS v4
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants