Polars change in enforcing non-NaN measurements for all turbines

[Bartdoekemeijer]

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:

flasc/flasc/energy_ratio/energy_ratio.py

Line 72 in 685871d

df_ = df_.filter(pl.all_horizontal(pl.col(ref_cols + test_cols + ws_cols + wd_cols).is_not_null()))

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:

flasc/flasc/floris_tools.py

Line 89 in 88330a7

mirror_nans=True,

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?

[misi9170]

Hi @Bartdoekemeijer, thanks for pointing this out. I agree with you, that requiring all reference (test) turbines to have a numerical value is overly restrictive, and we should at least have the option to require only that at least one of the reference (test) values is valid.

In terms of what the default behavior should be, I guess I could go either way:

• Requiring all by default is the most "predictable", in that the user will get exactly what they asked for when providing the list of reference (test) turbines. However, this may mean that a lot of data is filtered out.
• Requiring any by default is friendlier in terms of keeping data, but may be slightly more opaque to the user.

I don't imagine that this change should be too difficult; presumably we can use some sort of combination of not and pl.any_horizontal().is_not_null() to get the behavior you're suggesting. Or maybe the double negative wouldn't be needed :)

[paulf81]

Thanks for catching this @Bartdoekemeijer and I agree with you @misi9170 , we should just switch to any and this should work. I will open an issue on this and take responsibility for proposing a fix. I think we should also include a test or two to make sure the behavior of this is stable

[paulf81]

Opened Issue #120

[Bartdoekemeijer]

Thank you, both! Closing this discussion and continuing the conversation in #120.