Skip to content

irradiance.king documentation and output naming is wrong #2636

Closed

Description

@ramaroesilva

Describe the bug

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?

Screenshots
Equation from PVPMC:

Image

Versions:

  • pvlib.__version__: 0.13.1

Additional context
Add any other context about the problem here.

Activity

  1. changed the title [-]King model is wrongly defined[/-] [+]irradiance.king model documentation and output naming is wrong[/+] on Dec 21, 2025
  2. changed the title [-]irradiance.king model documentation and output naming is wrong[/-] [+]irradiance.king documentation and output naming is wrong[/+] on Dec 21, 2025
  3. cwhanse commented on Dec 21, 2025

    @cwhanse
    Member

    The documentation mistake was made in the original port from Matlab. The Matlab code is clear that the function returns total POA diffuse.

  4. ramaroesilva commented on Dec 21, 2025

    @ramaroesilva
    ContributorAuthor

    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.

  5. cwhanse commented on Dec 22, 2025

    @cwhanse
    Member

    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.

  6. ramaroesilva commented on Dec 22, 2025

    @ramaroesilva
    ContributorAuthor

    Have no idea how deprecation/deletion is done, so should I leave it for more experienced contributors to handle?

  7. cwhanse commented on Dec 22, 2025

    @cwhanse
    Member

    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.

  8. adriesse commented on Dec 22, 2025

    @adriesse
    Member

    I doubt it's in use anywhere

    Did someone use it in one of the PVPMC simulation round robins?

    (I'm ok with deprecating it either way.)

  9. kandersolar commented on Jan 13, 2026

    @kandersolar
    Member

    Have no idea how deprecation/deletion is done

    For simple functions like this, deprecating is pretty straightforward. #2495 is an example to follow.

  10. williamhobbs commented on Jun 19, 2026

    @williamhobbs
    Contributor

    I doubt it's in use anywhere

    Did someone use it in one of the PVPMC simulation round robins?

    (I'm ok with deprecating it either way.)

    @adriesse yes, it was used in Deville et al. 2024, https://doi.org/10.1002/pip.3763:

    Image Image
  11. ramaroesilva commented on Jun 21, 2026

    @ramaroesilva
    ContributorAuthor

    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.

  12. adriesse commented on Jun 23, 2026

    @adriesse
    Member

    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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    deprecationUse for issues and PRs which involve deprecations

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions