Implement Lorenz et al. POA and GHI QC methods #167

abhisheksparikh · 2022-11-13T04:55:20Z

Closes QA methods for plane-of-array irradiance #123
Added tests to cover all new or modified code.
Clearly documented all new API functions with PEP257 and numpydoc compliant docstrings.
Added new API functions to docs/api.rst.
Non-API functions clearly documented with docstrings or comments as necessary.
Adds description and name entries in the appropriate "what's new" file
in docs/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`).
Pull request is nearly complete and ready for detailed review.
Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

This is a draft PR. I need help editing the .rst files.

kandersolar

Thanks @abhisheksparikh, this is looking pretty good. This PR has some merge conflicts, largely due to editing the same files as #163 did. If you need help resolving those let us know.

Also, the reference says:

Additionally, we limit the maximal step between two consecutive irradiance measurements to 1000 W/m2 for both GHI and GTI as proposed for also in Espinar et al. (2011).

Do we need to do that as well in order to claim we have fully implemented the method as published?

pvanalytics/quality/irradiance.py

pvanalytics/tests/quality/test_irradiance.py

Co-authored-by: Kevin Anderson <kevin.anderson@nrel.gov>

docs/whatsnew/0.1.3.rst

pvanalytics/quality/irradiance.py

pvanalytics/tests/quality/test_irradiance.py

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

cwhanse · 2023-01-09T15:01:51Z

I think that flagging low irradiance at nighttime is OK in the context of pvanalytics. The package provides other functions which label day/night periods, and each quality function returns a time series of Boolean for the critieria it checks. The temptation is to regard 'True' here as equivalent to a valid measurement - and that is not always the case, as the discussion brings out.

irradiance measurements at nighttime typically have a slightly negative value

That is certainly true for thermopile instruments, but not for all instruments. I think a reference cell would report a small positive value.

AdamRJensen · 2023-03-20T17:40:52Z

Elke Lorenz has written the following to me:

For the naming: Do you think it would be o.k. to name the functions “pvlive” instead of “Lorenz”? I think that using the name of the main author of a paper is what you typically do, but this was really team work and so I think “pvlive” would be more appropriate.

Thus, I suggest that we change the name as requested. If anyone have any objection, please comment!

abhisheksparikh · 2023-03-20T18:45:23Z

Sounds good! Abhi

…

On Mon, Mar 20, 2023, 10:41 AM Adam R. Jensen ***@***.***> wrote: Elke Lorenz has written the following to me: For the naming: Do you think it would be o.k. to name the functions “pvlive” instead of “Lorenz”? I think that using the name of the main author of a paper is what you typically do, but this was really team work and so I think “pvlive” would be more appropriate. Thus, I suggest that we change the name as requested. — Reply to this email directly, view it on GitHub <#167 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG6HX5ENAP55SUS7R3Y3BN3W5CJDBANCNFSM6AAAAAAR6UZCRI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

kandersolar · 2023-05-25T15:52:24Z

Friendly ping @abhisheksparikh about this PR :)

docs/api.rst

pvanalytics/quality/irradiance.py

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

AdamRJensen

The code seems good, so well done!

My main concern is that it seems like an awful lot of code to implement the integer flags. I would prefer to just have the individual functions, but it's not a strong preference.

pvanalytics/quality/irradiance.py

AdamRJensen · 2023-05-30T12:31:04Z

pvanalytics/quality/irradiance.py

+    The upper limit for `poa_global` is set to 0 when `solar_zenith` is greater
+    than 90 degrees. Missing values of `poa_global`, `solar_zenith`


I'm still in strong favor of not flagging values at nighttime. I recently had a chat with the main author of the paper who did not have a recommendation for night time. Lorenz pointed me toward one of the co-authors who has yet to respond to my request.

Not having thought about the significance of nighttime data, I don't have a strong opinion on this. But if I were to think of irradiance as a part of operational analysis of a PV plant, I would like to have some distinction between daytime and nighttime data.

AdamRJensen · 2023-05-30T12:37:57Z

pvanalytics/quality/irradiance.py

+    # Changing the poa_global_flag to 3 when the step change in poa values is
+    # more than 1000 W/m2
+    poa_global_limit_int_flag = poa_global_limit_int_flag.mask(
+        poa_global.diff().abs() > 1000,
+        3
+    )


I think this deserves it's own function.

The step change limit is fairly common:

For 1-min data Shafer et al. (2000) recommended a step change limit of 800 W/m2, although Espinar (2011) suggested 1000 W/m2 for GHI. Lorenz et al. (2022) also suggested using the 1000 W/m2 threshold for tilted irradiance measurements.

It could be done in a separate pull request.

Good point. I think one good reason to have the step change limit in here is to stay true to the list of checks the paper suggests.

we could have a helper function that returns the step threshold, and pass it a kwarg 'pvlive' to return 1000.

However, I am completely fine with separating out the filter as a separate function ☺️.

As am I, let's do this and have a step_limit kwarg or something.

we could have a helper function that returns the step threshold, and pass it a kwarg 'pvlive' to return 1000.

Can somebody help me with this? After some reading, I think I understand kwargs, but have never written a function with them. Some more guidance on how the functions' design would be helpful.

@abhisheksparikh let's do that in a follow-on pull request. I feel like we are good enough here and we can improve later. Agree @AdamRJensen ?

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

abhisheksparikh · 2023-05-30T13:14:53Z

The code seems good, so well done!

My main concern is that it seems like an awful lot of code to implement the integer flags. I would prefer to just have the individual functions, but it's not a strong preference.

Do you have any recommendations? For e.g. would you combine the upper and lower limit functions? Individual functions provide clarity, but I am okay with combining them.

abhisheksparikh · 2023-05-30T14:08:02Z

The code seems good, so well done!

My main concern is that it seems like an awful lot of code to implement the integer flags. I would prefer to just have the individual functions, but it's not a strong preference.

Yeah, I agree, the code is definitely longer for what it's doing.

pvanalytics/quality/irradiance.py

cwhanse · 2023-05-30T15:12:08Z

@abhisheksparikh I think we should explain the integer flag values. I don't know if my suggestions will render properly as a web page.

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

AdamRJensen · 2023-06-08T14:49:37Z

The authors have responded to our question concerning what to do for nighttime values:

Regarding the night values: in our implementation we set all night values to zero and set a Flag with value 1 for all values changed. But actually, the method is not made for night values. So as you wrote, not checking is also an option which is maybe better for general applications. For some applications night values might be of interest, we actually do not use them anywhere.

So for the integer output, I think we should set nighttime values to 1 ~~True~~. But for the bool series returned, I suggest not setting any nighttime limits but rather setting a flag of True or False depending on the outcome of #191.

Additionally, the authors also wrote:

One other thing I have to point out. While checking your implementation we found a mistake in the publication, which is very unfortunate. We write I0 as solar constant with a fixed value. But actually I0 should be the Normal extraterrestrial irradiance corrected from Sun-Earth distance, which has a yearly cycle. We use the formula proposed by Spencer 1971 with a simplification suggested in “Energy Meteorology, Lecture Notes from Detlev Heinemann, 2002 Formula 2.16”

@abhisheksparikh This should be relatively straightforward to correct. You can look at the existing tests which do something very similar.

abhisheksparikh · 2023-06-14T04:51:51Z

So for the integer output, I think we should set nighttime values to True. But for the bool series returned, I suggest not setting any nighttime limits but rather setting a flag of True or False depending on the outcome of #191.

@AdamRJensen - I am confused by this: 'So for the integer output, I think we should set nighttime values to True'.

I will change the Normal extraterrestrial irradiance based on the formula.

AdamRJensen · 2023-06-14T22:39:45Z

@AdamRJensen - I am confused by this: 'So for the integer output, I think we should set nighttime values to True'.

A typo on my behalf - the flag for the integer should of course be 1 as stated by the authors 😄

abhisheksparikh added 4 commits November 11, 2022 19:19

implemented gpoa limit lorenz check

4175a4f

updated nomenclature and completed test functions

0fd23e3

edited description of a function

9399adf

updated documentation and api.rst

60861b2

abhisheksparikh marked this pull request as ready for review November 16, 2022 07:48

kperrynrel added the enhancement New feature or request label Nov 17, 2022

kperrynrel added this to the v0.1.3 milestone Nov 17, 2022

kandersolar reviewed Nov 28, 2022

View reviewed changes

abhisheksparikh and others added 2 commits November 28, 2022 12:00

style change

94d3430

Co-authored-by: Kevin Anderson <kevin.anderson@nrel.gov>

simplifying initiation of poa_global_limit_int_flag

792f10d

Co-authored-by: Kevin Anderson <kevin.anderson@nrel.gov>

kperrynrel self-requested a review November 28, 2022 20:05

abhisheksparikh and others added 2 commits November 28, 2022 12:05

Simplifying the initiation of poa_global_limit_bool_flag

a2aa469

Co-authored-by: Kevin Anderson <kevin.anderson@nrel.gov>

minor changes

7097952

kandersolar mentioned this pull request Dec 9, 2022

v0.1.3 planning #175

Closed

abhisheksparikh added 2 commits December 12, 2022 00:41

Added GHI check func and completed POA check func

97f3a9c

Merge branch 'main' into poa_qa

8d2007b

cwhanse reviewed Dec 12, 2022

View reviewed changes

abhisheksparikh and others added 13 commits December 12, 2022 09:00

Update docs/whatsnew/0.1.3.rst

a11d8d8

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

75dda23

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

6c56859

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

4da8913

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

7420547

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/tests/quality/test_irradiance.py

e98d476

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/tests/quality/test_irradiance.py

e0bfa2b

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/tests/quality/test_irradiance.py

d0591cc

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/tests/quality/test_irradiance.py

6d99699

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/tests/quality/test_irradiance.py

e25674b

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

895bf72

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

98f8445

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

3ddfcc9

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

AdamRJensen mentioned this pull request Mar 15, 2023

Account for lower GHI limit in check_irradiance_consistency_qcrad #188

Closed

abhisheksparikh added 2 commits May 25, 2023 23:16

Merge branch 'main' into poa_qa

165ce91

Changed the function name from Lorenz to pvlive

863ecde

AdamRJensen reviewed May 26, 2023

View reviewed changes

docs/api.rst Outdated Show resolved Hide resolved

pvanalytics/quality/irradiance.py Outdated Show resolved Hide resolved

pvanalytics/quality/irradiance.py Outdated Show resolved Hide resolved

abhisheksparikh and others added 3 commits May 26, 2023 10:56

Update docs/api.rst

57ae3bf

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

Update pvanalytics/quality/irradiance.py

901e0a0

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

Update pvanalytics/quality/irradiance.py

9e05e4c

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

AdamRJensen mentioned this pull request May 30, 2023

Values outside QC test domain are incorrectly flagged as in error #191

Open

AdamRJensen reviewed May 30, 2023

View reviewed changes

abhisheksparikh and others added 3 commits May 30, 2023 06:06

Update pvanalytics/quality/irradiance.py

771c721

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

Update pvanalytics/quality/irradiance.py

acb8a40

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

Update pvanalytics/quality/irradiance.py

368a6d0

Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>

cwhanse approved these changes May 30, 2023

View reviewed changes

pvanalytics/quality/irradiance.py Outdated Show resolved Hide resolved

pvanalytics/quality/irradiance.py Outdated Show resolved Hide resolved

pvanalytics/quality/irradiance.py Outdated Show resolved Hide resolved

pvanalytics/quality/irradiance.py Outdated Show resolved Hide resolved

cwhanse mentioned this pull request May 30, 2023

Add extreme BSRN QC limits #190

Open

4 tasks

abhisheksparikh and others added 4 commits June 3, 2023 22:15

Update pvanalytics/quality/irradiance.py

3e27f93

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

8764c7b

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

ee9f400

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

Update pvanalytics/quality/irradiance.py

74cac5a

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

kandersolar modified the milestones: v0.2.0, v0.2.1 Feb 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Lorenz et al. POA and GHI QC methods #167

Implement Lorenz et al. POA and GHI QC methods #167

abhisheksparikh commented Nov 13, 2022 •

edited

kandersolar left a comment

cwhanse commented Jan 9, 2023

AdamRJensen commented Mar 20, 2023 •

edited

abhisheksparikh commented Mar 20, 2023 via email

kandersolar commented May 25, 2023

AdamRJensen left a comment

AdamRJensen May 30, 2023

abhisheksparikh May 30, 2023

AdamRJensen May 30, 2023

AdamRJensen May 30, 2023 •

edited

abhisheksparikh May 30, 2023

cwhanse May 30, 2023

cwhanse May 30, 2023

abhisheksparikh Jun 4, 2023

cwhanse Jun 5, 2023

abhisheksparikh commented May 30, 2023

abhisheksparikh commented May 30, 2023

cwhanse commented May 30, 2023

AdamRJensen commented Jun 8, 2023 •

edited

abhisheksparikh commented Jun 14, 2023 •

edited

AdamRJensen commented Jun 14, 2023

		The upper limit for `poa_global` is set to 0 when `solar_zenith` is greater
		than 90 degrees. Missing values of `poa_global`, `solar_zenith`

Implement Lorenz et al. POA and GHI QC methods #167

Are you sure you want to change the base?

Implement Lorenz et al. POA and GHI QC methods #167

Conversation

abhisheksparikh commented Nov 13, 2022 • edited

kandersolar left a comment

Choose a reason for hiding this comment

cwhanse commented Jan 9, 2023

AdamRJensen commented Mar 20, 2023 • edited

abhisheksparikh commented Mar 20, 2023 via email

kandersolar commented May 25, 2023

AdamRJensen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamRJensen May 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhisheksparikh commented May 30, 2023

abhisheksparikh commented May 30, 2023

cwhanse commented May 30, 2023

AdamRJensen commented Jun 8, 2023 • edited

abhisheksparikh commented Jun 14, 2023 • edited

AdamRJensen commented Jun 14, 2023

abhisheksparikh commented Nov 13, 2022 •

edited

AdamRJensen commented Mar 20, 2023 •

edited

AdamRJensen May 30, 2023 •

edited

AdamRJensen commented Jun 8, 2023 •

edited

abhisheksparikh commented Jun 14, 2023 •

edited