use horner's method in disc

[mikofski]

XXX: don't merge before merging ASV benchmarks for disc() and perhaps functions that use disc(), a WIP

• Closes use horners method in DISC instead of calculating so many powers and polynomials #1180
• I am familiar with the contributing guidelines
• Tests added - existing unit tests are sufficient
• ASV benchmarks added - before merging this, it would be instructive to have ASV benchmarks to measure the change in runtime, perhaps also for functions that use DISC like GTI DIRINT
• Updates entries to docs/sphinx/source/api.rst for API changes. - unnecessary?
• Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
• New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary. - unnecessary?
• Pull request is nearly complete and ready for detailed review.
• Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

Speed up DISC by using Horner's method for polynomials. Don't merge. until ASV benchmarks have been merge to measure runtime of existing disc() method and other functions that use it

[mikofski]

Locally, this improves runtime for DISC (also used in GTI-DIRINT) by about 20% (from ~25ns/loop to 20ns/loop)

in "main":

>>> %timeit test_disc_value
# 27.4 ns ± 0.381 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)

>>> %timeit test_disc_overirradiance
# 25.6 ns ± 0.378 ns per loop (mean ± std. dev. of 7 runs, 10,000,000 loops each)

using Horner's

>>> %timeit test_disc_value
# 19.5 ns ± 0.526 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)

>>> %timeit test_disc_overirradiance
# 19.6 ns ± 0.815 ns per loop (mean ± std. dev. of 7 runs, 100,000,000 loops each)

[mikofski]

@pvlib/pvlib-maintainer I'm not sure this requires a what's new entry, it's a very minor change, but it does improve runtime for DISC which is used in GTI-DIRINT, but ~20% possibly.

[cwhanse]

I think we're ok here with a what's new entry

[cwhanse]

@mikofski I actually meant to write "without a whatsnew" but no harm in having it :)

Could you fix the stickler complaints?

[mikofski]

I like Anton’s idea in #1180 (comment):

Thinking about code clarity, you could consider changing the name bools to is_cloudy

anyone opposed?

[mikofski]

I changed bools to is_cloudy because kt<=0.6 means GHI < 0.6*ETRDNI*cos(solar_zenith) like a cloudy day:

diff --git a/pvlib/irradiance.py b/pvlib/irradiance.py
index 082370d..e916400 100644
--- a/pvlib/irradiance.py
+++ b/pvlib/irradiance.py
@@ -1481,15 +1481,15 @@ def _disc_kn(clearness_index, airmass, max_airmass=12):

     am = np.minimum(am, max_airmass)  # GH 450

-    # Use Horner's method to compure polynomials efficiently
-    bools = (kt <= 0.6)
-    a = np.where(bools,
+    is_cloudy= (kt <= 0.6)
+    # Use Horner's method to compute polynomials efficiently
+    a = np.where(is_cloudy,
             0.512 + kt*(-1.56 + kt*(2.286 - 2.222*kt)),
             -5.743 + kt*(21.77 + kt*(-27.49 + 11.56*kt)))
-    b = np.where(bools,
+    b = np.where(is_cloudy,
             0.37 + 0.962*kt,
             41.4 + kt*(-118.5 + kt*(66.05 + 31.9*kt)))
-    c = np.where(bools,
+    c = np.where(is_cloudy,
             -0.28 + kt*(0.932 - 2.048*kt),
             -47.01 + kt*(184.2 + kt*(-222.0 + 73.81*kt)))

[mikofski]

@pvlib/pvlib-core please review

[kandersolar]

@mikofski is there supposed to be a new asv benchmark included in the PR, or was that just for testing locally?

I changed bools to is_clear not is_cloudy because it is defined for kt<=0.6:

Doesn't low kt correspond to non-clear sky condition? Maybe you are still thinking about diffuse fraction from the Boland PR :P