Diffuse Self-Shading Example: Incorrect transposition of shading.sky_diffuse_passias

[kdebrab]

Describe the bug I think the second graph of https://pvlib-python.readthedocs.io/en/stable/gallery/shading/plot_passias_diffuse_shading.html is incorrect, even though it corresponds with the given reference.

In particular, transposing the shading loss in the horizontal plane shading.sky_diffuse_passias(psi) to the plane of array by multiplying it with a transposition ratio seems incorrect to me.

Instead the 'relative diffuse irradiance' (defined as 'ratio of diffuse plane of array irradiance (after accounting for shading) to diffuse horizontal irradiance') should be directly calculated as 1 - shading.sky_diffuse_passias(surface_tilt + psi).

This result also corresponds (more or less) with the result obtained when using the view factor method bifacial.utils.vf_row_sky_2d_integ(surface_tilt, gcr), which I guess is the most accurate method to calculate row-to-row diffuse shading.

The difference between the various methods is illustrated with the following graph:

from pvlib import bifacial, shading, irradiance
from cycler import cycler
import matplotlib.pyplot as plt
import numpy as np

surface_tilt = np.arange(0, 90, 0.5)

plt.rc('axes', prop_cycle=(cycler('color', ['b',  'orange', 'g', 'r']) * cycler('linestyle', ['-', ':', '--'])))
plt.figure()
for k in [1, 1.5, 2, 10]:
    gcr = 1/k
    psi = shading.masking_angle_passias(surface_tilt, gcr)
    shading_loss = shading.sky_diffuse_passias(psi)
    transposition_ratio = irradiance.isotropic(surface_tilt, dhi=1.0)
    passias_wrong = transposition_ratio * (1-shading_loss) * 100  # %
    passias_corrected = (1 - shading.sky_diffuse_passias(surface_tilt + psi)) * 100 # %
    vf_row_sky_2d_integ = bifacial.utils.vf_row_sky_2d_integ(surface_tilt, gcr) * 100 # %

    plt.plot(surface_tilt, passias_wrong, label=f"passias k={k}")
    plt.plot(surface_tilt, passias_corrected, label=f"passias corrected k={k}")
    plt.plot(surface_tilt, vf_row_sky_2d_integ, label=f"vf_row_sky_2d_integ k={k}")

plt.xlabel('Inclination angle [degrees]')
plt.ylabel('Relative diffuse irradiance [%]')
plt.ylim(0, 105)
plt.legend()
plt.show()

[kandersolar]

Thanks @kdebrab. I think more or less the same issue is described in #1825, do you agree?

IMHO this deserves a note in the passias functions' docstrings. Could also consider removing those functions, but maybe there's value in keeping it to help with investigations like the one in this issue.

[kdebrab]

@kandersolar Wauw, this is indeed basically the same issue as #1825. I had searched for any related issue but somehow search queries don't return questions from Q&A, even when removing is:issue from the query.

I agree that, apart from amending the self-shading example, a note in the docstrings of 'masking_angle_passias' and 'sky_diffuse_passias' is desired, (1) explaining correct usage and (2) hinting to the more precise bifacial.utils.vf_row_sky_2d_integ function (as it avoids the approximation of masking_angle_passias). Also, the description of the 'masking_angle' parameter of 'sky_diffuse_passias' should be updated that it actually needs the sum of masking and surface angle.

I wouldn't remove the functions, as they may indeed have valid use cases.

If OK for you, I will make a PR with the suggested changes (next week).

[kdebrab]

https://github.com/pvlib/pvlib-python/pull/2589 is ready for review. Wording can probably be improved in some places.

I wanted to recommend the alternative use of bifacial.utils.vf_row_sky_2d_integ in the docstring of sky_diffuse_passias, but felt that that's basically equivalent to deprecating the method, so I ended up adding an explicit (pending) deprecation to sky_diffuse_passias in favor of bifacial.utils.vf_row_sky_2d_integ.

[kandersolar]

I wanted to recommend the alternative use of bifacial.utils.vf_row_sky_2d_integ in the docstring of sky_diffuse_passias, but felt that that's basically equivalent to deprecating the method

Deprecation implies that we intend to remove the function eventually, so IMHO we don't want to deprecate anything here. I think we should note the issue and suggest an alternative.

(one reason to keep the function is that the model is used in SAM; see section 9.2 in https://docs.nrel.gov/docs/fy18osti/67399.pdf)