Accept float in PVSystem.get_irradiance

[cwhanse]

• [x] Closes #1338
• [X] I am familiar with the contributing guidelines
• [x] Tests added
• [ ] Updates entries in docs/sphinx/source/reference for API changes.
• [x] 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`).
• [x] New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
• [x] Pull request is nearly complete and ready for detailed review.
• [x] Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

If solar_zenith is not a Series with a DatetimeIndex, then the solar constant is used.

[cwhanse]

Working on this bug fix uncovers a related bug: the pvsystem.PVSystem.get_irradiance docstring says that irradiance parameters can be tuple of float, but that doesn't work:

system = pvsystem.PVSystem(surface_tilt=32, surface_azimuth=135)
irrads = {'dni': (900., 0.), 'ghi': (600., 0.), 'dhi': (100., 0.)}
zenith = 55.366831
azimuth = 172.320038
irradiance = system.get_irradiance(zenith,
                                   azimuth,
                                   irrads['dni'],
                                   irrads['ghi'],
                                   irrads['dhi'])

raises ValueError: Length mismatch for per-array parameter

So tuple of float of length N only works with a PVSystem with N Arrays.

I would like to edit the docstring to remove the promise that a tuple of floats will work. If more than one irradiance value is to be processed, they can be supplied in a Series. A tuple of Series, one Series for each Array, should still be accepted.

[kandersolar]

So tuple of float of length N only works with a PVSystem with N Arrays.

Isn't that the intended behavior? Tuples for these inputs are supposed to index over Arrays. That example:

irrads = {'dni': (900., 0.), 'ghi': (600., 0.), 'dhi': (100., 0.)}
irradiance = system.get_irradiance(..., irrads['dni'], irrads['ghi'], irrads['dhi'])

is simulating a single timestamp for system with two Arrays, where for some reason it is daytime for one Array and night for the other. The specific values are not realistic, but the use case is still valid (IMHO). I wouldn't call it a bug.

[cwhanse]

It would be simpler to describe behavior if we could state "tuples are distributed over Arrays". But that's not what happens when the system has only one Array: the tuple is treated the same as a Series of timestamped values.

If we want to keep the case of "tuple of float" I can put that back in the docstring as:

dni: float, tuple of float, Series or tuple of Series
    Direct normal irradiance [W/m2]

Notes
------
        Each of ``dni``, ``ghi``, and ``dni`` may be passed as a float, tuple of
        float, Series or tuple of Series. If passed as a float or Series, these
        values are used for every Array. If passed as a tuple of float and there
        is more than one Array, the tuple is distributed over the Arrays so the
        tuple length must be the number of Arrays. If passed as a tuple of Series
        the tuple is distributed over the Arrays and the tuple length must be
        the number of Arrays.

[kandersolar]

It would be simpler to describe behavior if we could state "tuples are distributed over Arrays". But that's not what happens when the system has only one Array: the tuple is treated the same as a Series of timestamped values.

Can you provide an example? I'm not seeing that behavior:

from pvlib.pvsystem import PVSystem
import pandas as pd

system = PVSystem(surface_tilt=32, surface_azimuth=135)

for converter in [pd.Series, tuple]:
    irradiance = system.get_irradiance(
        solar_zenith=converter([45, 45]),
        solar_azimuth=converter([180, 180]),
        dni=converter([900, 900]),
        ghi=converter([600, 600]),
        dhi=converter([100, 100])
    )
    print(irradiance)

[cwhanse]

Can you provide an example? I'm not seeing that behavior.

Neither am I. I must have been imagining what was originally intended by including "tuple of float" in the docstring.

To be sure we're seeing the same thing, that code produces ValueError: Length mismatch for per-array parameter for converter=tuple. Correct?

And this is the case we want to accomodate?

[kandersolar]

To be sure we're seeing the same thing, that code produces ValueError: Length mismatch for per-array parameter for converter=tuple. Correct?

Correct.

And this is the case we want to accomodate?

On the contrary, I think pvlib already behaves correctly. Tuples should get distributed over Arrays, so tuples of anything (float or Series) should only work when the length of the tuple equals the number of Arrays. The case above shows that a single-Array PVSystem raises an error when given tuples of length 2. That seems correct to me. So what I am saying is that I think the "related bug" described in https://github.com/pvlib/pvlib-python/pull/2227#issuecomment-2378029879 is actually the intended behavior.

[cwhanse]

Tuples should get distributed over Arrays, so tuples of anything (float or Series) should only work when the length of the tuple equals the number of Arrays.

We are in agreement now. Thanks.

[cwhanse]

@kandersolar I think I have addressed the review comments

[kandersolar]

Thanks @cwhanse