Replies: 4 comments 7 replies
-
|
hi @misi9170, thanks a lot for putting this all together in one place! I really appreciate the comprehensive review and links to the related issues and discussions! Suggestions
Other thoughts
|
Beta Was this translation helpful? Give feedback.
-
|
Perhaps for the 0 wind speed case, we could have the WindData objects detect if there is a 0 provided and bump it to 0+1e-6 or something? So, passing fmodel.set(wind_speeds[0, 1, 2], ...)
fmode.run()might still cause an error, but wind_rose = WindData(wind_speeds=[0, 1, 2, ...], ...)
fmodel.set(wind_data=wind_rose)
fmodel.run()would not? |
Beta Was this translation helpful? Give feedback.
-
|
I like this approach, it's simple and will work. And again fits a similar pattern, where the |
Beta Was this translation helpful? Give feedback.
-
|
on the inability to set ct to 0, if we had the |
Beta Was this translation helpful? Give feedback.
-
Well, the idea here is to come up with some general rules of thumb, so I think it would make sense for those to eventually be handled by an |
Beta Was this translation helpful? Give feedback.
-
|
Maybe challenging (1) a bit: why would you accept NaNs in the FLORIS inputs? Your code will go through the motions of evaluating all the expressions while knowing you will already get a NaN. And to avoid additional code to explicitly ignore NaN inputs, why not just raise an error if the user inserts a NaN value? This way, you just put it up to the user to deal with how to deal with NaNs. Regarding zero wind speeds,
Similar to inserting a NaN, why not raise a user error for zero and negative wind speeds? This puts the responsibility with the user and gives minimal obscurity. Understandable, it may be quite annoying to deal with the zero wind speed errors, so I like the proposed solution of fixing it in WindData with an epsilon, as long as there is a clear WARNING message that explains what's happening under the hood. |
Beta Was this translation helpful? Give feedback.
-
|
I'm also open to not accepting NaN values in FLORIS. Maybe we just have to pick one option even if both are defensible. On the 0 wind speed, here I think we should expect that FLORIS can compute this condition and so adding the epsilon is the most appealing option to me. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the input @Bartdoekemeijer ! To explain the reasoning behind my preference to allow NaNs to pass through FLORIS: basically, I'm just following numpy's lead. Since numpy operations allow NaNs to be passed through, unless the result is ambiguous, and since FLORIS is built heavily on numpy, my gut feeling is that FLORIS should also allow NaNs to pass through unless the result is ambiguous (e.g. the current way that This also gives power back to the user: the user is welcome to filter out NaN values from their inputs, or leave them in, depending on their preference. A good example of this is if the user has time series data with some NaN values in, say, the wind speed channel, and would like to compute the farm powers for that time series. They have a couple of options currently:
If we raise an error if a NaN is passed, that removes the third of these options, and users will be forced to remove NaNs from their inputs. That does mean that someone that doesn't know NaNs are present will now be made aware; but it removes flexibility for someone that does know and wants to proceed anyway. I guess my other feeling is that there is already quite a bit of bloat in FLORIS for "checks". That may be appropriate---it gives us the opportunity to tell users very explicitly what is going wrong---but it can also have unintended consequences, like the current situation where a NaN passed to
To be fair, those were warnings, not errors, and adding an error would probably have caused examples to fail (rather than to succeed, with a bunch of new warnings printed out), but still. Another thing to throw in here is that AI coding tools make it extremely easy to add a whole host of new Ok, all of this being said, I'm not actually vehemently opposed to the idea of raising an error for NaN inputs in the main FLORIS routine. We'd have to have NaN input checks added in various places ( |
Beta Was this translation helpful? Give feedback.
-
|
Thanks all! I just wanted to double check how things roughly work now so made this scripts: # Testing NaNs
import numpy as np
from floris import FlorisModel
# Set up two turbine FLORIS
fmodel = FlorisModel(configuration="defaults")
# Changing the wind farm layout uses FLORIS' set method to a two-turbine layout
fmodel.set(layout_x=[0, 500.0], layout_y=[0.0, 0.0])
# Baseline conditions
print("Running Baseline Conditions")
fmodel.set(
wind_directions=np.array([280.0]), wind_speeds=[8.0], turbulence_intensities=np.array([0.06])
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...Baseline Turbine Powers: {turbine_powers}")
# Wind direction is NaN
print("Running Wind Direction NaN")
fmodel.set(
wind_directions=np.array([np.nan]), wind_speeds=[8.0], turbulence_intensities=np.array([0.06])
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...WD NaN Turbine Powers: {turbine_powers}")
# Wind speed is NaN
print("Running Wind Speed NaN")
fmodel.set(
wind_directions=np.array([270.0]), wind_speeds=[np.nan], turbulence_intensities=np.array([0.06])
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...WS NaN Turbine Powers: {turbine_powers}")
# TI is NaN
print("Running TI NaN")
fmodel.set(
wind_directions=np.array([270.0]), wind_speeds=[8.0], turbulence_intensities=np.array([np.nan])
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...TI NaN Turbine Powers: {turbine_powers}")
# Yaw NaN
print("Running Yaw NaN")
fmodel.set(
wind_directions=np.array([270.0]),
wind_speeds=[8.0],
turbulence_intensities=np.array([0.06]),
yaw_angles=np.array([[0, np.nan]]),
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...YAW NaN Turbine Powers: {turbine_powers}")
# Power Setpoint NaN
print("Running Power Setpoint NaN")
fmodel.set_operation_model("mixed")
fmodel.reset_operation()
fmodel.set(
wind_directions=np.array([270.0]),
wind_speeds=[8.0],
turbulence_intensities=np.array([0.06]),
power_setpoints=np.array([[1e6, np.nan]]),
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...Power Setpoint NaN Turbine Powers: {turbine_powers}")
print("=============== 0 cases")
# Wind Speed 0
print("Running Wind Speed 0")
fmodel.reset_operation()
fmodel.set(
wind_directions=np.array([270.0]), wind_speeds=[0.0], turbulence_intensities=np.array([0.06])
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...WS = 0: {turbine_powers}")
# Power Setpoint 0
print("Running Power Setpoint 0")
fmodel.set(
wind_directions=np.array([270.0]),
wind_speeds=[8.0],
turbulence_intensities=np.array([0.06]),
power_setpoints=np.array([[1e6, 0]]),
)
fmodel.run()
turbine_powers = fmodel.get_turbine_powers() / 1000.0
print(f"...Power Setpoint = 0: {turbine_powers}")Which yields: Of the above I think the biggest concern is the wind direction = NaN case should not have been able to yield values. On the 0 cases, finally the results are ok so mostly we want to add the epsilon in both cases to hide the warnings right? Maybe I might finally say:
@misi9170 , @Bartdoekemeijer , @rafmudaf nearing consensus? |
Beta Was this translation helpful? Give feedback.
-
|
Interesting... thanks for pointing this out. Seems there's more silent NaN conversion going on than I knew about... |
Beta Was this translation helpful? Give feedback.
-
|
Indeed, very interesting that some NaNs still lead to numerical results. Yes, converging on consensus I can get with either:
Also I'm onboard with the epsilons to address 0.0 wind speed and power setpoints! |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
#1007 proposes adding some handling for NaN inputs passed to the
WindDataobjects. As part of the review of that PR, I dug a bit into what happens when NaNs are passed directly into thewind_speeds,wind_directions, etc ofFlorisModel.set(), and found results that are (possibly) surprising. Meanwhile, there is also an open question #1069 about what should happen when numeric values that are somehow "invalid" are passed into FLORIS, such as a 0 wind speed or a 0 power setpoint (something that @paulf81 mentioned recently causes NaN outputs---note that when we disable a turbine, we set the power setpoint to a small nonzero value). I am opening this discussion to detail FLORIS' current behavior with NaNs, have a discussion about expected behavior, and finally decide on guidelines for how FLORIS should handle NaNs and out-of-range values in future. NaN-handling behavior was also mentioned in #292 , but not discussed in detail.Current behavior
Currently, if the
wind_speedsargument toFlorisModel.set()contains any findices with aNaNvalue,FlorisModel.run()executes without issue, and a NaN power will appear at that findex inFlorisModel.get_farm_power(). However,.get_farm_AEP()produces a non-NaN value. For example,produces
In particular, note that
get_farm_powerisnan. This seems to me to be reasonable behavior.nanvalue is ignored in the numerator of the mean but not in the denominator). However, this is a special case wherefreqis generated automatically and uniformly; in reality, a providedfreqvalue would presumably not contain a nonzero value for a wind direction corresponding tonan.fmodel.set(layout_x=[0], layout_y=[0])) and one of thewind_directionsisnan, none of the output powers arenan. This is ok; the wind direction does not need to be known to compute the power if there is only one turbine. However, as expected, if one of thewind_speedsisnan, the corresponding farm power isnan. So, nothing too concerning there, but perhaps an interesting outcome.On the other hand, if
wind_speedspassed to theWindRoseobject contains NaNs, as described in #1007, a possibly obscure error is currently raised:ValueError: wind_speeds must be monotonically increasing.Suggestions
I propose that:
FlorisModel.set(). NaN powers should be produced, as is the current behavior, ifget_farm_powers()is called.get_farm_AEP()should produce NaN. The reason this doesn't currently happen is that we have anp.nansum()call. We should find anynp.nansum()ornp.nanmean()operations in thefloris.coremodule and think carefully about whether these should in fact benp.sum()ornp.mean(). If thenp.nansum()/nanmean()is still needed, the reason for this should be thoroughly documented.WindDataobjects. These objects are meant to make it easier for users to handle their input data, and raising errors for NaNs feels appropriate here.Notes
get_farm_AEP()works in the presence of NaNs).Beta Was this translation helpful? Give feedback.
All reactions