Code and example for classifying snow effects #209

cwhanse · 2024-04-08T15:01:05Z

Closes Quantify power losses due to snow cover #200
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.

…rize()

docs/examples/snow-detection/snow-mode.py

kandersolar

Another partial review. High-level thoughts:

The gallery example does too much. Simpler would be better. I think reducing it to a single inverter, or even a single combiner box, would be an improvement. I also think getting rid of unnecessary steps (e.g. horizon, SDM) would help.
I am a little uncomfortable with the functions being only loosely tied to the reference (at least by pvlib-python standards). The code both extends the reference (e.g. adding a new mode) and fills in many gaps where the reference doesn't give specifics. It seems that these changes are necessary, so maybe all we can do is document the differences.

docs/whatsnew/0.2.1.rst

docs/examples/snow-detection/snow-mode.py

kandersolar · 2024-09-27T14:30:42Z

pvanalytics/features/snow.py

+    .. [1] C. F. Abe, J. B. Dias, G. Notton, G. A. Faggianelli, G. Pigelet, and
+       D. Ouvrard, David, "Estimation of the effective irradiance and bifacial
+       gain for PV arrays using the maximum power current", IEEE Journal of
+       Photovoltaics, 2022, pp. 432-441. :doi:`10.1109/JPHOTOV.2023.3242117`


Suggested change

.. [1] C. F. Abe, J. B. Dias, G. Notton, G. A. Faggianelli, G. Pigelet, and

D. Ouvrard, David, "Estimation of the effective irradiance and bifacial

gain for PV arrays using the maximum power current", IEEE Journal of

Photovoltaics, 2022, pp. 432-441. :doi:`10.1109/JPHOTOV.2023.3242117`

.. [1] C. F. Abe, J. B. Dias, G. Notton, G. A. Faggianelli, G. Pigelet,

and D. Ouvrard, "Estimation of the effective irradiance and bifacial

gain for PV arrays using the maximum power current", IEEE Journal of

Photovoltaics, 2022, pp. 432-441. :doi:`10.1109/JPHOTOV.2023.3242117`

Something weird is happening with the rendering here. I am guessing sphinx is interpreting the leading C. and D. as indicating an enumerated list. I'm not sure, but I think this tweak (not having D. at the start of the second line) might get sphinx to render it as intended.

This comment is still unresolved

docs/examples/snow-detection/snow-mode.py

kandersolar · 2024-09-27T17:35:19Z

docs/examples/snow-detection/snow-mode.py

+        Keys are ``'transmission'``, ``'modeled_vmp'``, ``'vmp_ratio'``,
+        and ``'mode'``


These keys need to be updated according to what the function actually returns

Also, maybe return a dataframe instead of a dict?

This Returns description still needs to be updated

docs/examples/snow-detection/snow-mode.py

Suggestions from review Co-authored-by: Kevin Anderson <[email protected]>

cwhanse · 2024-09-27T22:03:24Z

Another partial review. High-level thoughts:

The gallery example does too much. Simpler would be better.

Thanks for this. I needed an outside perspective, and I will overhaul the example to handle data from a single combiner.

I am a little uncomfortable with the functions being only loosely tied to the reference (at least by pvlib-python standards). The code both extends the reference (e.g. adding a new mode) and fills in many gaps where the reference doesn't give specifics. It seems that these changes are necessary, so maybe all we can do is document the differences.

Agreed, but I'm not sure there's an option besides document here, or abandon the PR. A revised reference paper is very unlikely.

kandersolar · 2024-10-04T13:58:11Z

Test failure is in test_daytime.py; not caused by this PR. See pvlib/pvlib-python#2238

docs/api.rst

cwhanse · 2024-10-07T20:34:53Z

@kandersolar see if the example is reviewable now.

kandersolar

The example script is much more accessible now. It's still a bit on the long side, but maybe it has to be.

A few remaining comments (some of which refer to the previous review). Otherwise I think this PR is about ready to merge.

kandersolar · 2024-10-09T17:10:11Z

pvanalytics/features/snow.py

+    .. [1] C. F. Abe, J. B. Dias, G. Notton, G. A. Faggianelli, G. Pigelet, and
+       D. Ouvrard, David, "Estimation of the effective irradiance and bifacial
+       gain for PV arrays using the maximum power current", IEEE Journal of
+       Photovoltaics, 2022, pp. 432-441. :doi:`10.1109/JPHOTOV.2023.3242117`


This comment is still unresolved

kandersolar · 2024-10-09T17:10:44Z

docs/examples/snow-detection/snow-mode.py

+        Keys are ``'transmission'``, ``'modeled_vmp'``, ``'vmp_ratio'``,
+        and ``'mode'``


This Returns description still needs to be updated

kandersolar · 2024-10-09T17:18:03Z

pvanalytics/features/snow.py

+    imp0 : float
+        Short-circuit current at reference condition. [A]
+


irrad_ref is still undocumented

pvanalytics/tests/features/test_snow.py

cwhanse · 2024-10-11T21:19:49Z

thanks @kandersolar one last look or LGTY?

kandersolar · 2024-10-16T14:14:45Z

Yes I think it's time to merge this. Thanks @eccoope and @cwhanse for making this PR happen!

cwhanse and others added 30 commits April 8, 2024 08:59

initial load of snow mode detection

455a1ae

lint

096d480

fix file opens

bfe3642

Merge branch 'main' of https://github.com/pvlib/pvanalytics into snow

14caa28

add README

c0d47b3

formatting

eae4a84

fix import

83592a2

fix file name

ac12f36

fix up horizon code, add test

556fefc

fix horizon file name

025b74b

update mask function

acf0b4e

linting

0b8720d

work toward vectorizing categorize

1f7a0d6

lint, use mode None instead of -1

e4e2219

use None in test

fc2fd20

use dtype object to have both int and None

8a73eef

handle mixed types

32619fb

add tests

42e582a

handle equals

2af3fa0

Fixed the horizon masking section

9a1494c

Linting

3cc7c55

Added argument to test_categorize() to reflect changes made to catego…

a4465b7

…rize()

last lint i promise!

4ad80cd

remove old categorize

cb67705

Fixed failing test

6de642c

Fixed failing test

409a29e

fix offline

c0d0a3f

Merge branch 'snow' of https://github.com/cwhanse/pvanalytics into snow

6e93d0d

0 not None

ccfac03

edit definitions of mode

0d5c087

cwhanse added 8 commits September 9, 2024 07:46

put Bug fixes back in whatsnew

46f089d

fix merge

6a7420a

catchup with github changes

e7be3a7

doc improvements

741e721

save the file first

35cbb7c

adjust colors

ba1694f

figure nidges

335478c

figure nidges

be3eef5

cwhanse commented Sep 10, 2024

View reviewed changes

docs/examples/snow-detection/snow-mode.py Outdated Show resolved Hide resolved

Update docs/examples/snow-detection/snow-mode.py

fff5452

kandersolar reviewed Sep 27, 2024

View reviewed changes

cwhanse and others added 3 commits September 27, 2024 12:16

small edits

9dbc990

Merge branch 'snow' of https://github.com/cwhanse/pvanalytics into snow

4075739

Apply suggestions from code review

8cd1fca

Suggestions from review Co-authored-by: Kevin Anderson <[email protected]>

overhaul example, remove horizon

9335a74

cwhanse added 5 commits October 7, 2024 08:45

Merge branch 'main' of https://github.com/pvlib/pvanalytics into snow

72f5066

edits, I forget what

1d221ca

major edits to example

7c80d3a

yet more edits

87da65c

lint

5356804

cwhanse commented Oct 7, 2024

View reviewed changes

docs/api.rst Outdated Show resolved Hide resolved

Update docs/api.rst

f9d9638

kandersolar approved these changes Oct 9, 2024

View reviewed changes

review comments

d45be6a

kandersolar merged commit 308c6d2 into pvlib:main Oct 16, 2024
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code and example for classifying snow effects #209

Code and example for classifying snow effects #209

cwhanse commented Apr 8, 2024 •

edited

Loading

kandersolar left a comment

kandersolar Sep 27, 2024

kandersolar Oct 9, 2024

kandersolar Sep 27, 2024

kandersolar Sep 27, 2024

kandersolar Oct 9, 2024

cwhanse commented Sep 27, 2024

kandersolar commented Oct 4, 2024

cwhanse commented Oct 7, 2024

kandersolar left a comment •

edited

Loading

kandersolar Oct 9, 2024

kandersolar Oct 9, 2024

kandersolar Oct 9, 2024

cwhanse commented Oct 11, 2024

kandersolar commented Oct 16, 2024

		Keys are ``'transmission'``, ``'modeled_vmp'``, ``'vmp_ratio'``,
		and ``'mode'``

		imp0 : float
		Short-circuit current at reference condition. [A]

Code and example for classifying snow effects #209

Code and example for classifying snow effects #209

Conversation

cwhanse commented Apr 8, 2024 • edited Loading

kandersolar 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhanse commented Sep 27, 2024

kandersolar commented Oct 4, 2024

cwhanse commented Oct 7, 2024

kandersolar left a comment • edited Loading

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

cwhanse commented Oct 11, 2024

kandersolar commented Oct 16, 2024

cwhanse commented Apr 8, 2024 •

edited

Loading

kandersolar left a comment •

edited

Loading