From 8b16bb38953137dca698665f9dbeb342a02e63e9 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Fri, 21 Jul 2023 10:46:39 -0400 Subject: [PATCH 1/5] docstring improvements --- pvlib/iam.py | 44 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/pvlib/iam.py b/pvlib/iam.py index ca8f89468e..5531892a00 100644 --- a/pvlib/iam.py +++ b/pvlib/iam.py @@ -800,7 +800,7 @@ def schlick(aoi): In PV contexts, the Schlick approximation has been used as an analytically integrable alternative to the Fresnel equations for estimating IAM - for diffuse irradiance [2]_. + for diffuse irradiance [2]_ (see :py:func:`schlick_diffuse`). Parameters ---------- @@ -845,9 +845,20 @@ def schlick_diffuse(surface_tilt): ground-reflected irradiance on a tilted surface using the Schlick incident angle model. - The diffuse iam values are calculated using an analytical integration - of the Schlick equation [1]_ over the portion of an isotropic sky and - isotropic foreground that is visible from the tilted surface [2]_. + The Schlick equation (or "Schlick's approximation") [1]_ is an + approximation to the Fresnel reflection factor which can be recast as + a simple photovoltaic IAM model like so: + + .. math:: + + IAM = 1 - (1 - \cos(aoi))^5 + + Unlike the Fresnel reflection factor itself, Schlick's approximation can + be integrated analytically to derive a closed-form equation for diffuse + IAM factors for the portions of the sky and ground visible + from a tilted surface. This function implements an integration of the + Schlick approximation provided by Xie et al. [2]_ which assumes isotropic + sky and foreground. Parameters ---------- @@ -873,6 +884,31 @@ def schlick_diffuse(surface_tilt): Renewable and Sustainable Energy Reviews, vol. 161, 112362. June 2022. :doi:`10.1016/j.rser.2022.112362` + Notes + ----- + The analytical integration of the Schlick approximation was derived + as part of the FEDIS diffuse IAM model [2]_. Compared with the model + implemented in this function, the FEDIS model includes an additional term + to account for reflection off a pyranometer's glass dome. Because that + reflection should already be accounted for in the instrument's calibration, + the pvlib authors believe it is incorrect to account for it again in an + IAM model. Thus, this function omits that term and implements only + the integration of the Schlick approximation. + + Note also that the output of this function (which is an exact integration) + can be compared with the output of numerical integration of the Schlick + approximation using :py:func:`marion_diffuse`: + + .. code:: + + >>> pvlib.iam.marion_diffuse('schlick', surface_tilt=20) + {'sky': 0.9625000227247358, + 'horizon': 0.7688174948510073, + 'ground': 0.6267861879241405} + + >>> pvlib.iam.schlick_diffuse(surface_tilt=20) + (0.9624993421569652, 0.6269387554469255) + See Also -------- pvlib.iam.schlick From 9d405bd0c08f05527691148f28d9a37d64c422de Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Fri, 21 Jul 2023 10:49:33 -0400 Subject: [PATCH 2/5] whatsnew --- docs/sphinx/source/whatsnew/v0.10.2.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/sphinx/source/whatsnew/v0.10.2.rst b/docs/sphinx/source/whatsnew/v0.10.2.rst index 93dc230d15..db733c1884 100644 --- a/docs/sphinx/source/whatsnew/v0.10.2.rst +++ b/docs/sphinx/source/whatsnew/v0.10.2.rst @@ -27,6 +27,8 @@ Testing Documentation ~~~~~~~~~~~~~ +* Added docstring detail for :py:func:`pvlib.iam.schlick_diffuse`. + (:issue:`1811`, :pull:`1812`) * Removed Stickler-CI integration as the service has ceased June 2023. (:issue:`1722`, :pull:`1723`) From bf1ff5dbd822d66bfef1dfc0920acd977eb80b38 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Fri, 21 Jul 2023 11:03:23 -0400 Subject: [PATCH 3/5] correct docstring section order --- pvlib/iam.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/pvlib/iam.py b/pvlib/iam.py index 5531892a00..e6ef8ef20d 100644 --- a/pvlib/iam.py +++ b/pvlib/iam.py @@ -813,6 +813,10 @@ def schlick(aoi): iam : numeric The incident angle modifier. + See Also + -------- + pvlib.iam.schlick_diffuse + References ---------- .. [1] Schlick, C. An inexpensive BRDF model for physically-based @@ -822,10 +826,6 @@ def schlick(aoi): for Diffuse radiation on Inclined photovoltaic Surfaces (FEDIS)", Renewable and Sustainable Energy Reviews, vol. 161, 112362. June 2022. :doi:`10.1016/j.rser.2022.112362` - - See Also - -------- - pvlib.iam.schlick_diffuse """ iam = 1 - (1 - cosd(aoi)) ** 5 iam = np.where(np.abs(aoi) >= 90.0, 0.0, iam) @@ -874,15 +874,9 @@ def schlick_diffuse(surface_tilt): iam_ground : numeric The incident angle modifier for ground-reflected diffuse. - References - ---------- - .. [1] Schlick, C. An inexpensive BRDF model for physically-based - rendering. Computer graphics forum 13 (1994). - - .. [2] Xie, Y., M. Sengupta, A. Habte, A. Andreas, "The 'Fresnel Equations' - for Diffuse radiation on Inclined photovoltaic Surfaces (FEDIS)", - Renewable and Sustainable Energy Reviews, vol. 161, 112362. June 2022. - :doi:`10.1016/j.rser.2022.112362` + See Also + -------- + pvlib.iam.schlick Notes ----- @@ -909,9 +903,15 @@ def schlick_diffuse(surface_tilt): >>> pvlib.iam.schlick_diffuse(surface_tilt=20) (0.9624993421569652, 0.6269387554469255) - See Also - -------- - pvlib.iam.schlick + References + ---------- + .. [1] Schlick, C. An inexpensive BRDF model for physically-based + rendering. Computer graphics forum 13 (1994). + + .. [2] Xie, Y., M. Sengupta, A. Habte, A. Andreas, "The 'Fresnel Equations' + for Diffuse radiation on Inclined photovoltaic Surfaces (FEDIS)", + Renewable and Sustainable Energy Reviews, vol. 161, 112362. June 2022. + :doi:`10.1016/j.rser.2022.112362` """ # these calculations are as in [2]_, but with the refractive index # weighting coefficient w set to 1.0 (so it is omitted) From f04103c7fb56eb5e3f388f43fc12018467e7fec0 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Fri, 21 Jul 2023 11:53:43 -0400 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Cliff Hansen --- pvlib/iam.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pvlib/iam.py b/pvlib/iam.py index e6ef8ef20d..5be443e442 100644 --- a/pvlib/iam.py +++ b/pvlib/iam.py @@ -885,13 +885,13 @@ def schlick_diffuse(surface_tilt): implemented in this function, the FEDIS model includes an additional term to account for reflection off a pyranometer's glass dome. Because that reflection should already be accounted for in the instrument's calibration, - the pvlib authors believe it is incorrect to account for it again in an - IAM model. Thus, this function omits that term and implements only - the integration of the Schlick approximation. + the pvlib authors believe it is inappropriate to account for pyranometer + reflection again in an IAM model. Thus, this function omits that term and + implements only the integrated Schlick approximation. Note also that the output of this function (which is an exact integration) - can be compared with the output of numerical integration of the Schlick - approximation using :py:func:`marion_diffuse`: + can be compared with the output of :py:func:`marion_diffuse` which numerically + integrates the Schlick approximation: .. code:: From a1ad502cf3b02b9c1637a59c4f1adf46dcc7718f Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Tue, 25 Jul 2023 09:39:02 -0400 Subject: [PATCH 5/5] Update pvlib/iam.py Co-authored-by: Anton Driesse --- pvlib/iam.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pvlib/iam.py b/pvlib/iam.py index 5be443e442..b02d20989b 100644 --- a/pvlib/iam.py +++ b/pvlib/iam.py @@ -856,9 +856,9 @@ def schlick_diffuse(surface_tilt): Unlike the Fresnel reflection factor itself, Schlick's approximation can be integrated analytically to derive a closed-form equation for diffuse IAM factors for the portions of the sky and ground visible - from a tilted surface. This function implements an integration of the - Schlick approximation provided by Xie et al. [2]_ which assumes isotropic - sky and foreground. + from a tilted surface if isotropic distributions are assumed. + This function implements the integration of the + Schlick approximation provided by Xie et al. [2]_. Parameters ----------