Open AdamRJensen opened 13 hours ago
Should a deprecation-warning period be established? I'm not aware of the internal methodology you guys make use of, but I've seen that changing params have been deprecated before in this project. See #773.
The previous definition was also not correct. IMO it doesn't you can't use the term "broadband" about effective irradiance which has potentially been modified for spectral effects.
g_poa_effective;broadband plane of array effective irradiance.
If you apply a dimensionless spectral factor correction to a value of broadband irradiance in Wm⁻², it's still a broadband value in Wm⁻² but just a fraction of the original broadband value. Broadband just means its the irradiance between a (wide) wavelength range, no?
Should a deprecation-warning period be established? I'm not aware of the internal methodology you guys make use of, but I've seen that changing params have been deprecated before in this project. See #773.
This would be ideal. But when it's just a rename I don't see how it can be deprecated nicely.
I don't think I agree with this change of name g_poa_effective
to effective_irradiance
. The current description is of a quantity that is not poa_global
but also is not effective_irradiance
which is supposed to be after accounting for reflections, soiling, and spectrum. I agree that the description can be improved.
Just a comment about names: sometimes variable names were chosen to more closely connect the code with the text in the reference. Didn't happen here (the reference uses :math:I_{tr}
so I'm guessing the submitter of this code came up with g_poa_effective
as a compromise :math:G_{POA}
being common in literature, and adding "_effective" because reflections have been removed.
g_poa_effective: numeric
effective_irradiance: numeric
Irradiance transmitted to the PV cells. To be
fully consistent with PVWatts, the user must have already
applied angle of incidence losses, but not soiling, spectral,
etc. [W/m^2]
This would be ideal. But when it's just a rename I don't see how it can be deprecated nicely.
@AdamRJensen , we can work it out, similarly to #773 with some little variations. Hint: signature will need to be:
def pvwatts_dc(self, effective_irradiance=None, temp_cell=None, *, g_poa_effective=None):
and the warning when it's used:
if g_poa_effective:
warnings.warn("Use 'effective_irradiance' to suppress this warning", pvlibDeprecationWarning)
effective_irradiance=g_poa_effective
tests of mutual exclusion
then the warning test.
because reflections have been removed
If I understand it well, then [post_]iam_irradiance
, iam_modified_irradiance
, irradiance_after_iam
, poa_global_after_iam
, poa_g_after_iam
may be options that fit the description better. What do you think about it @cwhanse ?
I'm +1 to changing the parameter name and description, +1 to using a self-explanatory parameter name, +1 for poa_global_after_iam
if I had to choose, not a strong opinion.
The current description is of a quantity that is not
poa_global
but also is noteffective_irradiance
which is supposed to be after accounting for reflections, soiling, and spectrum. I agree that the description can be improved.
We already have a definition of effective_irradiance
, which includes both reflection and spectral effects:
https://github.com/pvlib/pvlib-python/blob/4cfda4a14366217ba9bb3b15d2531c61b7507e69/pvlib/pvsystem.py#L2309-L2311
So the definition proposed here (AOI correction only) would conflict with the existing definition above.
I was just reading through the pvwatts manual now --- page 9 eqn 8 --- perhaps we could just just use the same "transmitted" terminology here instead, i.e. transmitted_irradiance
?
"transmitted" terminology here instead, i.e. transmitted_irradiance?
The De Soto model paper uses the term "absorbed irradiance" for this quantity, symbols $S$ and $S_{ref}$
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`
).remote-data
) and Milestone are assigned to the Pull Request and linked Issue.g_poa_effective
is an alternative name foreffective_irradiance
. The latter is the standard in pvlib (at least the most widely used).This is a breaking change, but I don't see how this can be deprecated in a nice way.
The previous definition was also not correct. IMO it doesn't you can't use the term "broadband" about effective irradiance which has potentially been modified for spectral effects.