Simplify/DRY some infinite_sheds/bifacial.utils calculations

[kandersolar]

• [ ] Closes #xxxx
• 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.

Projected solar zenith angle (tan_phi) is calculated several times in the infinite_sheds workflow. This PR calculates it once and passes it around, simplifying the relevant utility functions and their signatures.

For context, this is a step towards future enhancements I intend to make to pvlib.bifacial, which would benefit from working with tan_phi instead of the base solar position and module angles.

[echedey-ls]

You've probably got the email, but docs are failing with:

This failing example

Extension error: Here is a summary of the problems encountered when running the examples: Unexpected failing examples (1): ../../examples/agrivoltaics/plot_agrivoltaics_ground_irradiance.py failed leaving traceback: Traceback (most recent call last): File "/home/docs/checkouts/readthedocs.org/user_builds/pvlib-python/checkouts/2559/docs/examples/agrivoltaics/plot_agrivoltaics_ground_irradiance.py", line 145, in <module> unshaded_ground_fraction = pvlib.bifacial.utils._unshaded_ground_fraction( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TypeError: _unshaded_ground_fraction() got an unexpected keyword argument 'surface_azimuth'

My opinion is that it's bad practice to use internals in public-facing examples. But a fix, using internals or not, is okay with me.

Also, I think this deserves a whatsnew 👀
Nice improvement.

[kandersolar]

My opinion is that it's bad practice to use internals in public-facing examples.

Agreed. We should consider making that ground shaded fraction function public.

[kandersolar]

Closing as it's nicer to use phi instead of tan_phi. Will pursue a similar refactor in a later PR.

[mikofski]

You’re gonna hate this, but you could memoize tan_phi in a class and reevaluate lazily as needed when inputs update. Crazy right. So over the top. But maybe…?

[echedey-ls]

You’re gonna hate this, but you could memoize tan_phi in a class and reevaluate lazily as needed when inputs update. Crazy right. So over the top. But maybe…?

Easily hatable TBF 😆

The easiest option relies on python's lru_cache wrapper. At least this one, and I'm convinced many others too, use a hashmap (plain map in python) where input variables hash->cached return value. Numpy arrays can't be hashed, as they are mutable (plus CPython's default hash implementation is way too basic to work in this case). Also, if you want to bypass this with your suggestion (or other approach, like a wrapper), you would need to check the values of an array matches the values of a stored copy of the array in the cache. So you would end up consuming another significant amount of time.

I'm onboard with Kevin on this approach hehe. Thou that was a good concern @mikofski :)