Add extreme QCRAD QC limits #190
Conversation
+1 to this. What do you think about extending the output to a tuple (boolean series, integer series) as is being done in #167?
On reading that paper again, I agree. We can deprecate the existing functions and use _bsrn for functions that introduce the option to evaluate extremely rare value limits. We can address #191 in the new functions also. |
I'm not sure I see the advantage of having two outputs?
Yay! 😄 |
|
@cwhanse this year and a half old PR is ready for review |
pvanalytics/quality/irradiance.py
Outdated
| @@ -289,7 +314,7 @@ def check_irradiance_consistency_qcrad(solar_zenith, ghi, dhi, dni, | |||
| param=None, outside_domain=False): | |||
| r"""Check consistency of GHI, DHI and DNI using QCRad criteria. | |||
|
|
|||
| Uses criteria given in [1]_ to validate the ratio of irradiance | |||
| Uses criteria given in [1]_, [2]_ to validate the ratio of irradiance | |||
There was a problem hiding this comment.
Are the consistency criteria the same in both papers?
There was a problem hiding this comment.
[1] only gives one set, but [2] gives both sets. So one might be inclined to only reference [2]. However, [2] is merely a two-page document whereas [1] is a journal article. I have been in correspondence with Christian Gueymard, who recommended referring to both sets of coefficients as QCRad tests as they were developed in this framework.
There was a problem hiding this comment.
For the consistency function, I don't see any point in referencing [2]. Seems like it doesn't offer anything extra over the journal article.
Co-authored-by: Cliff Hansen <[email protected]>
|
@kandersolar ready for review! |
pvanalytics/quality/irradiance.py
Outdated
| @@ -289,7 +314,7 @@ def check_irradiance_consistency_qcrad(solar_zenith, ghi, dhi, dni, | |||
| param=None, outside_domain=False): | |||
| r"""Check consistency of GHI, DHI and DNI using QCRad criteria. | |||
|
|
|||
| Uses criteria given in [1]_ to validate the ratio of irradiance | |||
| Uses criteria given in [1]_, [2]_ to validate the ratio of irradiance | |||
There was a problem hiding this comment.
For the consistency function, I don't see any point in referencing [2]. Seems like it doesn't offer anything extra over the journal article.
Co-authored-by: Kevin Anderson <[email protected]>
[ ] Closes #xxx[ ] Added tests to cover all new or modified code.[ ] Added new API functions todocs/api.rst.[ ] Non-API functions clearly documented with docstrings or comments as necessary.in
docs/whatsnewfor all changes. Includes link to the GitHub Issue with
:issue:`num`or this Pull Request with
:pull:`num`. Includes contributor nameand/or GitHub username (link with
:ghuser:`user`).The QCRad software relies on the BSRN irradiance limits. Specifically, QCRad uses the "physical possible" BSRN limits for the upper limit but both the "physical possible" and the "extremely rare" BSRN limits for the lower limits. Currently, pvanalytics only support the "physical possible" limits.
However, the "extremely rare" limits are widely used, e.g., by Espinar et al. (2011), Geuder et al. (2015), Lorenz et al. (2022), and Forstinger et al. (2021).
In this PR I propose adding the "extremely rare" BSRN limits to the QCRad functions. Ideally, I think the functions should be named "_bsrn and not "*_qcrad" as QCRad states that it relies on the BSRN limits. At the current stage, the PR isn't breaking anything, but changing the function names would of course be a braking change.