[BUGFIX] Update CubatureGrid for 4-dimensional data structures

[rafmudaf]

Update CubatureGrid for 4-dimensional data structures

In #764, the core data structures in FLORIS were changed from 5-dimensional to 4-dimensional by collapsing the two left-most dimensions. Some parts of the CubatureGrid class were updated then, but it was not fully changed. Since this grid-type is not included in the automated tests, the error was not caught.

Separately, the check for a valid grid resolution was incorrect, so that has also been updated here.

This change can be verified to work with the following script. Before this change, the script raises an error.

from floris import FlorisModel
fmodel = FlorisModel("inputs/gch.yaml")
solver_settings = {
    "type": "turbine_cubature_grid",
    "turbine_grid_points": 3
}
fmodel.set(solver_settings=solver_settings)
fmodel.run()

turbine_powers = fmodel.get_turbine_powers()
print(turbine_powers.flatten())

Related issue

Tangentially related to #709.

Impacted areas of the software

CubatureGrid class

[rafmudaf]

@Bartdoekemeijer heads up that this was an issue and will be fixed. However, it does not address #709.

[Bartdoekemeijer]

Thanks @rafmudaf. Also, the cubature scheme generates a quadratic number of points similar to the default square turbine grid method, so 2x2, 3x3, 4x4. However, in the 3x3 setting for example, there are only 7 unique points. I spoke with Misha before about this, and we argued that perhaps a further simplification in FLORIS could be

(n_wind_directions, n_wind_speeds, n_rotor_points)

rather than
(n_wind_directions, n_wind_speeds, turbine_grid_n, turbine_grid_n)

This would allow an easy speed-up of almost 30% when using the cubature method without any loss in accuracy.

[rafmudaf]

@Bartdoekemeijer It's a good point, and @misi9170 also mentioned it to me. I agree that this change is worthwhile. However, for the cubature grid, we keep one dimension flat and the other contains the points. For N=3, it's (n findex, n turbines, 9, 1) in the v4 form, so collapsing the dimensions wouldn't solve the problem. Instead, we can remove any redundant points at the center and combines their weights. This pull request is only to fix the bug so that it will at least run, but I've also opened #883 where we can continue to talk through the remaining items to finalize this.

[misi9170]

Hi @rafmudaf , I think this looks good and I've confirmed that your example script runs on this branch but fails on develop, as you said.

This does beg the question though---should we be adding a test for the cubature grid, perhaps just something very simple based on your example script?

Also, even though we're in this period after the release of v4.0 where we are intending to get all bug fixes into main, I feel it's a little cleaner to merge these changes into develop and then fast-forward main to develop, rather than merging direct to main and then back merging those into develop?

[rafmudaf]

Hi @misi9170,

Thanks for reviewing these changes. I have the infrastructure for some tests in my fork's cubature_testing branch, but I haven't taken the time to think through good tests for this feature. My idea was to fix the bug in the code that exists in v4.0, and then add tests and validate for v4.1 as described in #883. However, I can add tests to this pull request before merging, if you'd rather.

And no worries on the target branch, I'll change it to develop.

[misi9170]

Hi @misi9170,

Thanks for reviewing these changes. I have the infrastructure for some tests in my fork's cubature_testing branch, but I haven't taken the time to think through good tests for this feature. My idea was to fix the bug in the code that exists in v4.0, and then add tests and validate for v4.1 as described in #883. However, I can add tests to this pull request before merging, if you'd rather.

And no worries on the target branch, I'll change it to develop.

Sounds good, thanks!