NREL / flasc

A rich floris-driven suite for SCADA analysis
https://nrel.github.io/flasc/
BSD 3-Clause "New" or "Revised" License
31 stars 18 forks source link

Change behavior for dealing with NaN/Null measurements #120

Closed paulf81 closed 1 year ago

paulf81 commented 1 year ago

Change behavior for dealing with NaN/Null measurements

Code changed inadvertently in pandas->polars conversion. Propose to submit a pull request which reverts to former behavior for dealing with NaNs on turbines within a set of reference or test turbines. Specifically:

Discussed in https://github.com/NREL/flasc/discussions/119

Originally posted by **Bartdoekemeijer** September 5, 2023 The recent Polars merge in `develop` has added a specific change to how NaNs are dealt with. When FLASC relied on Pandas, we defined `pow_ref` as the `nanmean` of the reference turbines, thus where NaNs were ignored and we just took the average of all non-NaN values. The same holds for `pow_test`. This means that if one reference/test turbine had a NaN value, that turbine would just be ignored and the power reference/test was just calculated with the remaining turbines. Now with Polars, FLASC enforces that the measurements of all reference turbines must be valid. The same thing holds for all test turbines. This is achieved in `energy_ratio.py` and in `energy_ratio_output.py` through the command: https://github.com/NREL/flasc/blob/685871d62ec7b311f54eccbcee48cb853b568bef/flasc/energy_ratio/energy_ratio.py#L72 I think this is particularly (and unnecessarily) restrictive. Note that it's fine to keep NaNs in our energy ratio analysis, so as long as those NaNs are mirrored in the FLORIS predictions, so that we're still comparing apples to apples. This in the case by default in `interpolate_floris_from_df_approx`, see: https://github.com/NREL/flasc/blob/88330a767d1a91dff49fcef39ea7d1ce67ae526d/flasc/floris_tools.py#L89 My solution to this would be to either remove the `is_not_null()` filter command in `energy_ratio.py` and in `energy_ratio_output.py`, or just turn it into an option that is disabled by default (my preference). I could probably be persuaded to enable it by default. What do you think, @paulf81, @misi9170?
paulf81 commented 1 year ago

Resolved by #121