Change g_poa_effective to effective_irradiance

[AdamRJensen]

• Related to variables_style_rules.csv could use an update #1253
• 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.

g_poa_effective is an alternative name for effective_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.

g_poa_effective;broadband plane of array effective irradiance.

[echedey-ls]

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.

[RDaxini]

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?

[AdamRJensen]

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.

[cwhanse]

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]

[echedey-ls]

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.

[RDaxini]

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.

We already have a definition of effective_irradiance, which includes both reflection and spectral effects:

pvlib-python/pvlib/pvsystem.py

Lines 2309 to 2311 in 4cfda4a

	effective_irradiance : numeric
	Effective irradiance accounting for reflections and spectral content.
	[W/m2]

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?

[cwhanse]

"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}$

[kandersolar]

If this parameter name changes, we should consider doing it in combination with broader changes to that function (name and module, see #1350 and #1544 (comment)).

PVWatts v5 doesn't account for spectrum (so you could say implicitly $M=1$). Soiling is applied as a generic loss factor after DC power is modeled. I agree that effective_irradiance is not quite right for the quantity in question, but that bothers me less than inventing/adopting a new term would. I suggest renaming to effective_irradiance and listing the caveats in the parameter description.

[AdamRJensen]

If this parameter name changes, we should consider doing it in combination with broader changes to that function (name and module, see #1350 and #1544 (comment)).

PVWatts v5 doesn't account for spectrum (so you could say implicitly M = 1 ). Soiling is applied as a generic loss factor after DC power is modeled. I agree that effective_irradiance is not quite right for the quantity in question, but that bothers me less than inventing/adopting a new term would. I suggest renaming to effective_irradiance and listing the caveats in the parameter description.

+1

[cwhanse]

I suggest renaming to effective_irradiance and listing the caveats in the parameter description.

The distinction between poa_global and effective_irradiance is probably not important to typical users of pvwatts_dc. If we are choosing to rename g_poa_effective to one of the already-used terms, I would prefer poa_global rather than effective_irradiance.

Although I'll point out the irony of this conversation at the same time we are discussing various instances of inconsistent parameter names.

[adriesse]

I would vote for effective_irradiance because that's consistent with the other DC module models/functions.

[RDaxini]

(not sure if my earlier comment was holding this up so, just in case, a follow up comment...)

In general I am not keen on reusing an existing variable with a different definition (comment), but in this specific case I think @kandersolar's comment

(so you could say implicitly M = 1 )

addresses the overlap to a sufficient extent so I'll leave my vote neutral (not opposed) on effective_irradiance