You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
irradiance.king documentation and output naming is wrong #2636
The King model is unclear and not even correct. Tbh, this is to be expected as the pvlib documentation irradiance.king explicitly assumes:
Note that this model is not well documented and has not been published in any fashion (as of January 2012).
The equation for King's model in PVPMC's webpage describes an isotropic transposition for both sky and ground diffuse assuming a zenith-dependent albedo. However, within pvlib the irradiance.king this is not made clear and it even mentions
"ground reflected irradiance is not included in this algorithm"
And while the output is poa_diffuse, it is identified as poa_sky_diffuse.
Expected behavior
Would propose that documentation and variable naming is corrected. Could also split the poa in sky and ground. @cwhanse proposed here that due to lack of proper documentation this function could be deprecated.
Would it be worth to include the ground diffuse implementation within irradiance.get_ground_diffuse?
changed the title [-]irradiance.king model documentation and output naming is wrong[/-][+]irradiance.king documentation and output naming is wrong[/+]on Dec 21, 2025
Personally, I would focus this function in its ground diffuse part (it's its only differentiating element). Changed my mind, the zenith-corrected albedo is negative when zenith <3º and achieves values like 0.68 for zenith=60º...
Would either deprecate it or at least correct the docstring and variable naming (maybe even including a note about the albedo with what I mentioned above). If it is kept, it will not be compatible with improvements like #2527, as, differently from other transposition functions, it computes the sum of sky and ground poa diffuse.
I agree, I think the intent was to adjust the isotropic model, not to represent ground diffuse separately.
I'm OK retiring this function. Deprecate now, delete on next+1 major release. I doubt it's in use anywhere (except perhaps in some internal Sandia code). If someone later requests it again, it can be re-implemented.
Originally, I was up for converting king to a ground reflection function, since it offers a differentiated function (to which is summed the isotropic for the sky diffuse). But I think @cwhanse argued in favor of it being deprecated due to lack of solid references (which is reasonable).
With @cbcrespo GSoC activities, don't know if it could make sense to have it return the total diffuse and split by components (although it is the only transposition function we have that returns both sky and ground) - complemented with a note on documentation that sky is fully treated as isotropic.
I think I would suggest removing it, also on the basis of questionable validity. The zenith-dependent albedo varies from -0.04 at zenith angle 0 to 1.04 at zenith angle 90, increasing even further into the twilight when you still might be measuring some GHI.
(EDIT: I see the albedo values were already criticized above.)
However, I think if is disappears without a trace left in the future latest documentation, our observations will be lost. Perhaps we could find a way to leave a permanent record in readthedocs?
Activity
[-]King model is wrongly defined[/-][+]irradiance.king model documentation and output naming is wrong[/+][-]irradiance.king model documentation and output naming is wrong[/-][+]irradiance.king documentation and output naming is wrong[/+]cwhanse commentedon Dec 21, 2025
The documentation mistake was made in the original port from Matlab. The Matlab code is clear that the function returns total POA diffuse.
ramaroesilva commentedon Dec 21, 2025
Personally,
I would focus this function in its ground diffuse part (it's its only differentiating element).Changed my mind, the zenith-corrected albedo is negative when zenith <3º and achieves values like 0.68 for zenith=60º...Would either deprecate it or at least correct the docstring and variable naming (maybe even including a note about the albedo with what I mentioned above). If it is kept, it will not be compatible with improvements like #2527, as, differently from other transposition functions, it computes the sum of sky and ground poa diffuse.
cwhanse commentedon Dec 22, 2025
I agree, I think the intent was to adjust the isotropic model, not to represent ground diffuse separately.
I'm OK retiring this function. Deprecate now, delete on next+1 major release. I doubt it's in use anywhere (except perhaps in some internal Sandia code). If someone later requests it again, it can be re-implemented.
ramaroesilva commentedon Dec 22, 2025
Have no idea how deprecation/deletion is done, so should I leave it for more experienced contributors to handle?
cwhanse commentedon Dec 22, 2025
pvlib has a deprecated decorator. It does seem like a bit of black magic to me, but if you'd like to try, here's an example.
adriesse commentedon Dec 22, 2025
Did someone use it in one of the PVPMC simulation round robins?
(I'm ok with deprecating it either way.)
kandersolar commentedon Jan 13, 2026
For simple functions like this, deprecating is pretty straightforward. #2495 is an example to follow.
williamhobbs commentedon Jun 19, 2026
@adriesse yes, it was used in Deville et al. 2024, https://doi.org/10.1002/pip.3763:
ramaroesilva commentedon Jun 21, 2026
Originally, I was up for converting
kingto a ground reflection function, since it offers a differentiated function (to which is summed the isotropic for the sky diffuse). But I think @cwhanse argued in favor of it being deprecated due to lack of solid references (which is reasonable).With @cbcrespo GSoC activities, don't know if it could make sense to have it return the total diffuse and split by components (although it is the only transposition function we have that returns both sky and ground) - complemented with a note on documentation that sky is fully treated as isotropic.
adriesse commentedon Jun 23, 2026
I think I would suggest removing it, also on the basis of questionable validity. The zenith-dependent albedo varies from -0.04 at zenith angle 0 to 1.04 at zenith angle 90, increasing even further into the twilight when you still might be measuring some GHI.
(EDIT: I see the albedo values were already criticized above.)
However, I think if is disappears without a trace left in the future latest documentation, our observations will be lost. Perhaps we could find a way to leave a permanent record in readthedocs?