Rename solar position inputs in tracking.singleaxis

[AdamRJensen]

• Closes tracking.singleaxis takes apparent_azimuth #2479
• I am familiar with the contributing guidelines
• Tests added
• 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.

Rename the inputs to the tracking.singleaxis function. Specifically, apparent_zenith -> solar_zenith and apparent_azimuth --> solar_azimuth

[echedey-ls]

LGTM :)
apparent_zenith has a lot of occurrences so I've not reviewed them all. apparent_azimuth only seems to exist as positional arguments in test_tracking.py. I don't think they deserve to change.

[kandersolar]

Glad to see this change finally happening!

Docs build looks clean, but the CI found one more in the tests:

pvlib-python/tests/test_tracking.py

Lines 296 to 297 in dfafe6d

	result = tracking.singleaxis(
	apparent_zenith=80, apparent_azimuth=338, axis_tilt=30,

Also can we add a new test using with pytest.warns(pvlibDeprecationWarning, match='...') for the new messages? Seems like good practice, even if somewhat redundant.

[cwhanse]

I'm ok with this change apparent_azimuth --> solar_azimuth but I note that the scope from #2479 has crept to include renaming apparent_zenith to solar_zenith. I'm OK with this renaming on the condition that the definition of solar_zenith in pvlib.tracking.singleaxis no longer specifies that solar_zenith is the apparent zenith. If we really mean that the singleaxis function should operate on apparent zenith, then the parameter should not be renamed.

There's another instance of apparent_azimuth, output from the pyephem solar position function, which could be removed.
https://github.colasdn.workers.dev.mcas-gov.ms/pvlib/pvlib-python/blob/main/pvlib/solarposition.py#L659

[mikofski]

This:

If we really mean that the singleaxis function should operate on apparent zenith, then the parameter should not be renamed.

IMHO it would be a grave disservice to imply that true extraterrestrial unrefracted zenith is correct in tracking algorithms, because it would lead to errors in tracker rotations and therefore incur significant losses in deployed PV systems.

Therefore I’m -1 on this change, if that’s the condition.

[williamhobbs]

I may be a little confused about what's being proposed and discussed, but I'm understanding that there are 3 items:

1. pvlib.tracking.singleaxis has an input that is confusingly named apparent_azimuth; it's confusing because unrefracted azimuth is the same as refracted azimuth, so "apparent" is meaningless
2. pvlib.tracking.singleaxis has another input named apparent_zenith. That's not necessarily bad, because trackers should use refracted (apparent) zenith in setting position.
3. Maybe that input of apparent_zenith should be renamed for consistency to something like solar_zenith, but then it should be clear to users that the typical (correct?) input for solar zenith here is apparent zenith.

The concern I have is that the outputs of pvlib solar position functions include apparent_zenith and zenith. Here, zenith means unrefracted zenith (right?). Then if we later have a term/parameter solar_zenith, it is unclear if that means a) the use-specific best version of zenith, b) unrefracted zenith, or c) apparent zenith. I think @mikofski is assuming solar_zenith as being proposed is unrefracted zenith (option b), but I think it means the best version (option a) which is apparent (option c).

[kandersolar]

In case folks missed it or have forgotten, there is useful and relevant discussion in #1403 of what solar_zenith should refer to. My read of that discussion was that there was popular but not unanimous support for solar_zenith to refer to refracted zenith. I'm still +1 to that, myself.

Note that if we demand that functions which take refracted zenith call the corresponding parameter apparent_zenith, then we'll need to change other functions (notably, irradiance.perez and other transposition models).

IMHO it would be a grave disservice to imply that true extraterrestrial unrefracted zenith is correct in tracking algorithms, because it would lead to errors in tracker rotations and therefore incur significant losses in deployed PV systems.

I was curious how big this error actually is so I ran a quick simulation. Unrefracted sun position is lower in the sky, so tracking (/backtracking) according to that will not cause self-shading. The difference in annual irradiation due to worse AOI is pretty small, only ~0.05% assuming always-clear conditions. Quick simulation code here:

Click to expand!

import pandas as pd
import pvlib

times = pd.date_range("2020-01-01", "2020-12-31 23:59", freq="1min", tz="Etc/GMT+5")
location = pvlib.location.Location(40, -80)

sp = location.get_solarposition(times)
cs = location.get_clearsky(times, solar_position=sp)
am = pvlib.atmosphere.get_relative_airmass(sp.apparent_zenith)
et = pvlib.irradiance.get_extra_radiation(times)

tr1 = pvlib.tracking.singleaxis(sp.apparent_zenith, sp.azimuth, backtrack=True, gcr=0.4)
tr2 = pvlib.tracking.singleaxis(sp.zenith, sp.azimuth, backtrack=True, gcr=0.4)

kwargs = dict(solar_zenith=sp.apparent_zenith, solar_azimuth=sp.azimuth, airmass=am, dni_extra=et, **cs)
gti1 = pvlib.irradiance.get_total_irradiance(tr1.surface_tilt, tr1.surface_azimuth, **kwargs)
gti2 = pvlib.irradiance.get_total_irradiance(tr2.surface_tilt, tr2.surface_azimuth, **kwargs)

rel_diff_annual_irrad = 1 - gti2['poa_global'].sum() / gti1['poa_global'].sum()

print(f"difference in annual GTI: {100 * rel_diff_annual_irrad:0.02f}%")

[williamhobbs]

Thanks, @kandersolar. I did either miss or forget about #1403. I'm also +1 for solar_zenith to refer to refracted zenith.

[mikofski]

I agree with Cliff & Will, I propose we limit scope in this PR to just renaming the confusing apparent_azimuth argument and move handling various ambiguous “zenith” terms in a separate issue so that it can have a dedicated discussion where we can hash out all the options Will pointed out. @williamhobbs suggestion has a precedent, the airmass and transposition functions use solar zenith then explain which “methods” use true zenith vs apparent. Eg:

• relative airmass see the notes section which methods use refracted zenith
• get_total _irradiance doesn’t have any guidance, the user just has to know to check the detailed method docs to learn whether to use refracted zenith or not. Eg: see perez. But no links are provided to these detailed method docs, and actually there is no transposition method that does not require refracted (aka “apparent”) zenith.

So this is a topic that deserves its own issue in my opinion