Adapts irradiance.isotropic to return_components framework.

[ramaroesilva]

• [ ] Closes #1553
• [x] I am familiar with the contributing guidelines
• [ ] Tests added
• [ ] Updates entries in docs/sphinx/source/reference for API changes.
• [ ] 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.
• [ ] Pull request is nearly complete and ready for detailed review.
• [ ] Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

[ramaroesilva]

For now, I only updated irradiance.isotropic as a testbed for the proposed changes.

Made use of the content from perez and haydaviesdocstrings, which were not fully consistent. Also, added the poa_ prefix to the outputs as suggested by @markcampanelli here

If this seems good to everyone, I can replicate this for all functions (adding the return_components and making documentation more consistent across fuctions, whenever each is needed).

[ramaroesilva]

Once more people give feedback on the documentation structuring of this example, I will continue developing the other transposition functions taking into account the OrderedDict comment. All poa_ related changes will move to a separate PR.

[ramaroesilva]

Sorry for letting this get stale, work got hectic.

Would like to follow up on @kandersolar initiative who is already including the poa_ prefix in the outputs in #2627 and try to push this PR (after expanding it to other transposition functions) for v0.14. Now we have the freedom of being allowed to introduce breaking changes.

Previous points made:

• @cwhanse would prefer including both poa_sky_diffuse and diffuse_components if return_components is True.
• @adriesse suggests to include only the lowest level and leave the summation up to the user.

Within a modelchain, in theory, the optical losses would use the components, whereas the module temperature most often uses the non-effective GTI. If only the components are kept, I guess that a summation would be needed somewhere within the modelchain.

Keeping both the sum and the components seems easier, otherwise we would need to have the code adapt to the user choice regarding return_components.

[ramaroesilva]

If we end up deciding to keep both sum and components, I was thinking if a single dict/DataFrame variable should be produced whether return_components is True or False (i.e., adding more keys/columns to the same base structure).

Pros:

• keeps same function calling in both cases (addressing one of @cwhanse points in this discussion)
• avoids duplicating indices in case of DataFrame

Cons:

• would need to adjust functions in modelchain (and get_total_irradiance maybe) that were receiving either the sum and components
• would be sending unnecessary data when other functions getting this input ask only for sum/components (increased overhead?)

Could also call the variable as poa_sky_diffuse and have the sum allocate to a total key.

[markcampanelli]

@ramaroesilva Here's my idea, which probably breaks too many things. Immutability opens up caching the sum, for a significant speedup.

"""
Example of a frozen dataclass for PAO diffuse components using cache for components sum.
"""

from dataclasses import dataclass
import functools
import typing

import numpy.typing


@dataclass(frozen=True)
class PoaDiffuseComponents:
    """Contanier for POA diffuse irradiance components."""

    poa_isotropic: numpy.typing.NDArray[numpy.floating[typing.Any]]
    poa_circumsolar: numpy.typing.NDArray[numpy.floating[typing.Any]]
    poa_horizon: numpy.typing.NDArray[numpy.floating[typing.Any]]

    @functools.cached_property
    def poa_sky_diffuse(self) -> numpy.typing.NDArray[numpy.floating[typing.Any]]:
        """Return sum of all POA diffuse components."""

        return self.poa_isotropic + self.poa_circumsolar + self.poa_horizon

    @property
    def isotropic(self) -> numpy.typing.NDArray[numpy.floating[typing.Any]]:
        """Return deprecated POA isotropic diffuse-component."""

        return self.poa_isotropic

    @property
    def circumsolar(self) -> numpy.typing.NDArray[numpy.floating[typing.Any]]:
        """Return deprecated POA circumsolar-diffuse component."""

        return self.poa_circumsolar

    @property
    def horizon(self) -> numpy.typing.NDArray[numpy.floating[typing.Any]]:
        """Return deprecated POA horizon-diffuse component."""

        return self.poa_horizon

    @property
    def sky_diffuse(self) -> numpy.typing.NDArray[numpy.floating[typing.Any]]:
        """Return deprecated sum of all POA diffuse components."""

        return self.poa_sky_diffuse


if __name__ == "__main__":

    import time

    rng = numpy.random.default_rng()

    poa_diffuse_components = PoaDiffuseComponents(
        poa_isotropic=rng.uniform(low=0.0, high=1100.0, size=(8760,)),
        poa_circumsolar=rng.uniform(low=0.0, high=1100.0, size=(8760,)),
        poa_horizon=rng.uniform(low=0.0, high=1100.0, size=(8760,)),
    )

    start_time_ns = time.perf_counter_ns()
    poa_diffuse_components.poa_sky_diffuse
    end_time_ns = time.perf_counter_ns()
    print(f"poa_sky_diffuse uncached time: {end_time_ns - start_time_ns} ns")

    start_time_ns = time.perf_counter_ns()
    poa_diffuse_components.poa_sky_diffuse
    end_time_ns = time.perf_counter_ns()
    print(f"poa_sky_diffuse cached time: {end_time_ns - start_time_ns} ns")

Example output:

poa_sky_diffuse uncached time: 24667 ns
poa_sky_diffuse cached time: 250 ns

[kandersolar]

I was thinking if a single dict/DataFrame variable should be produced whether return_components is True or False (i.e., adding more keys/columns to the same base structure).

For the sake of argument, if a dict/DataFrame is being returned no matter what, I'd suggest we always return the components so that we can simplify the API by getting rid of the return_components parameter. The extra memory would be minor, I expect.

Some relevant discussion here: https://github.com/pvlib/pvlib-python/issues/434

I'm not sure what I think, but I lean towards keeping the current behavior to minimize disruption.

[adriesse]

• @adriesse suggests to include only the lowest level and leave the summation up to the user.

Thanks for keeping track of my suggestion. I think it would be cleaner that way, and summation could be done trivially without knowing or using any of the column names.