NREL / flasc

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

[Bugfix] calc_florix_approx_table keyword inputs in examples #60

Closed misi9170 closed 1 year ago

misi9170 commented 1 year ago
Problem:

In examples/raw_data_processing/a_07a... and a_08..., there are calls to calc_floris_approx_table (from flasc/floris_tools.py) that are passed invalid keyword arguments num_workers and num_threads. This may have been from a previous implementation of calc_floris_approx_table that had or was intended to have parallel computing capabilities.

Steps to reproduce:
  1. Remove df_fi_approx.ftr from examples/raw_data_processing/, if it exists.
  2. Run a_07a_estimate_wind_direction_bias_per_turbine.py
Solution:

I've removed the invalid keyword arguments from the call. The code now runs as expected.

misi9170 commented 1 year ago

This goes back to commit 5d23e3b3, when the approach of calc_floris_approx_table changed away from calling calc_floris (which does use parallelization)

Bartdoekemeijer commented 1 year ago

Good catch @misi9170! Your PR looks good to me. Also, two more comments:

  1. People can still use calc_floris_approx_table with parallelization by just inserting a ParallelComputingInterface FLORIS object instead of a FlorisInterface object into it. See https://github.com/NREL/floris/pull/555 on the FLORIS repository. (I generally am really happy with how this ParallelComputingInterface class is working out -- it really just swaps out with a FlorisInterface object and then performs everything in parallel in the background.)
  2. I'm thinking about removing or at least seriously revising the calc_floris function since I don't use it anywhere in my code, currently.
paulf81 commented 1 year ago

Thank you @misi9170 and @Bartdoekemeijer !

misi9170 commented 1 year ago

@Bartdoekemeijer Thanks, that's a good hint on the parallel computing FLORIS interface and sounds good regarding possible deprecation of calc_floris.

misi9170 commented 1 year ago

Squashing and merging.