[BUGFIX] Tilt cosine law correction

[mkrause-strath]

Bugfix for tilt cosine law correction

The cosine law correction is currently applied relative to the reference tilt of the turbine which is incorrect. Instead it should be applied according to the following expression:

$v_{rotoreffective} = ( \frac{cos(\theta_{tilt})}{cos(\theta_{reftilt})})^{pP_{tilt}/3}$

See the full discussion here: #1124

Checks

• Ran a test example locally to ensure that negative tilt angles result in the same lost power as the equivalent positive tilt angle
• Did not run isort since no new imports
• Did not run ruff since in my environment this corrected almost all files
• Ran all tests and they passed

[misi9170]

Thanks for submitting this @mkrause-strath ! I'm working through it now. It's currently failing some of the tests, but I think this is appropriate and expected since the results in tilt should change. I'm going to make sure that everything is making sense there and possibly add one or two more tests; I'll push those to this branch. Once I've done that, I'll ask someone else for a review and we should be able to get this merged in.

EDIT: the tests were indeed passing locally, and it's not really to do with the change to the tilt that caused the failure. I believe what happened is that cos(x)/cos(x) is evaluating to something that is not precisely 1 (numerical error), which causes a numerical difference in a test result that is supposed to be "equal". I have now allowed a numerical tolerance to be accepted, and I think the test will now pass.

EDIT 2: I ended up also implementing the test bugfix in #1136, which is now merged. I've merged develop back into this branch, so that should no longer be an issue for this PR or future PRs.

However, I'd still like to add a couple of tests to check that the (new) tilt behavior is working as expected.

[mkrause-strath]

Interesting. I think I actually hit the same error on the yaw optimization test initially before making a fork. But then I also hit that same error just on the main branch if I remember correctly. I checked since I was suspicious that a tilt change would break a yaw optimization test.

After I made the fork I didn't hit it so it must just be intermittent or exact environment dependent.

[misi9170]

Yeah, this is not quite as simple as I thought---I believe there is some numerical instability in the yaw optimization regression test with scipy that can make the optimizer move to a different (local) optimum. I'll look into this some more and see if I can figure out what's going on.

[misi9170]

Scrap that, it was actually fairly simple---the numerical difference was causing the optimizer to settle at a slightly different location. The pd.testing.assert_frame_equal() call needed the tolerances provided explicitly after check_exact=False was specified though, because the default tolerances were still too tight. After relaxing the tolerances some (in particular the absolute tolerance, atol, which I reduced to only requiring a match up to 2 decimal places) the regression tests pass again. I'm satisfied that that is simply a numerical change caused by the change to cos(x) / cos(x) as described above.

[mkrause-strath]

@misi9170 Whilst looking at #1055 it has just occurred to me that the thrust calculation also wrongly uses the "relative tilt angle". Happy to fix this as well though bit busy until next Friday finishing my thesis.

[misi9170]

Good catch @mkrause-strath ! I think you're talking about this line. I agree; this should also be updated according to the same argument. Yes, feel free to update that too when you have time.

[mkrause-strath]

@misi9170 I have updated the way axial induction and thrust coefficient are calculated in the cosine-loss operation model. I have also updated some of the tests. However, some of the tests are still broken. Namely:

• empirical_gauss_regression_test.py
• turbine_multi_dim_unit_test.py (test_ct() and test_axial_induction())

I think that's because these teste use hard-coded values for assertion. I'm unsure on the method for updating these tests.

Another question. The expression cosd(tilt) or cosd(tilt_angles) is also used in some other places without any mention of the reference tilt. Examples:

• https://github.com/NREL/floris/blob/a6fc5d35aa32614edc450dc399c42af60a816887/floris/core/turbine/controller_dependent_operation_model.py#L104
• https://github.com/NREL/floris/blob/a6fc5d35aa32614edc450dc399c42af60a816887/floris/core/wake_deflection/gauss.py#L144
• https://github.com/NREL/floris/blob/a6fc5d35aa32614edc450dc399c42af60a816887/floris/core/wake_velocity/empirical_gauss.py#L148

I don't understand those operation/wake models enough to judge whether that's correct. So just asking the question to make sure this is fine?

[misi9170]

Thanks @mkrause-strath ! I'll take a look through this. Yes, for the regression tests, the comparisons are hardcoded by design---so I'll need to check that we expect those to change, and if they do, I'll update them. I can also look at the other places you pointed out. I need to look a bit more at the first one, but my gut reaction is that the later ones are currently correct---the wake development depends on the absolute value of the tilt angle, not whether the tilt angle is at the reference value or otherwise.

[misi9170]

Hi @mkrause-strath , I've now updated the regression tests.

Looking through the places you've called out, I think the tilt angles should be used as they are. For the first, this is in the ControllerDependentTurbine model, which uses a very different approach for calculating losses due to tilt and yaw than the CosineLossTurbine. Reading around the nearby code, I think the absolute tilt angle is still appropriate there.

The latter two are indeed to do with wake formation rather than modifying prespecified curves, which depends on the absolute tilt, so I'm happy with how those are.

I think this PR is now ready for final review and merger.

P.S. Congratulations on your Masters! :)

[mkrause-strath]

Hi @mkrause-strath , I've now updated the regression tests.

Looking through the places you've called out, I think the tilt angles should be used as they are. For the first, this is in the ControllerDependentTurbine model, which uses a very different approach for calculating losses due to tilt and yaw than the CosineLossTurbine. Reading around the nearby code, I think the absolute tilt angle is still appropriate there.

The latter two are indeed to do with wake formation rather than modifying prespecified curves, which depends on the absolute tilt, so I'm happy with how those are.

I think this PR is now ready for final review and merger.

P.S. Congratulations on your Masters! :)

Thank you!

Also thanks for checking out those other places and updating the tests.