-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Townsend snow #1251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Townsend snow #1251
Changes from all commits
0b736d4
3f2c40d
974d516
655fa79
c0c8b4d
f0e1f3a
78716f7
5e06a40
a5c5315
f79e906
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,7 +5,7 @@ | |||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import numpy as np | ||||||||||||||||||||||||||
import pandas as pd | ||||||||||||||||||||||||||
from pvlib.tools import sind | ||||||||||||||||||||||||||
from pvlib.tools import sind, cosd, tand | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def _time_delta_in_hours(times): | ||||||||||||||||||||||||||
|
@@ -185,3 +185,108 @@ def dc_loss_nrel(snow_coverage, num_strings): | |||||||||||||||||||||||||
Available at https://www.nrel.gov/docs/fy18osti/67399.pdf | ||||||||||||||||||||||||||
''' | ||||||||||||||||||||||||||
return np.ceil(snow_coverage * num_strings) / num_strings | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def _townsend_Se(S, N): | ||||||||||||||||||||||||||
''' | ||||||||||||||||||||||||||
Calculates effective snow for a given month based upon the total snowfall | ||||||||||||||||||||||||||
received in a month in inches and the number of events where snowfall is | ||||||||||||||||||||||||||
greater than 1 inch | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Parameters | ||||||||||||||||||||||||||
---------- | ||||||||||||||||||||||||||
S : numeric | ||||||||||||||||||||||||||
Snowfall in inches received in a month [in] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
N: numeric | ||||||||||||||||||||||||||
Number of snowfall events with snowfall > 1" [-] | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1" here too |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Returns | ||||||||||||||||||||||||||
------- | ||||||||||||||||||||||||||
effective_snowfall : numeric | ||||||||||||||||||||||||||
Effective snowfall as defined in the townsend model | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
References | ||||||||||||||||||||||||||
---------- | ||||||||||||||||||||||||||
.. [1] Townsend, Tim & Powers, Loren. (2011). Photovoltaics and snow: An | ||||||||||||||||||||||||||
update from two winters of measurements in the SIERRA. Conference | ||||||||||||||||||||||||||
Record of the IEEE Photovoltaic Specialists Conference. | ||||||||||||||||||||||||||
003231-003236. :doi:`10.1109/PVSC.2011.6186627` | ||||||||||||||||||||||||||
Available at https://www.researchgate.net/publication/261042016_Photovoltaics_and_snow_An_update_from_two_winters_of_measurements_in_the_SIERRA | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
''' # noqa: E501 | ||||||||||||||||||||||||||
return(np.where(N > 0, 0.5 * S * (1 + 1/N), 0)) | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised stickler doesn't complain about this. |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
def loss_townsend(snow_total, snow_events, surface_tilt, relative_humidity, | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "_townsend" or "_townsend_powers"? Often a paper will be cited as "X and Y" if only two authors, "X et al" if 3 or more. Not committed to that pattern here, just providing context. @mikofski? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a tough call, but I believe most folks already refer to this model as the "Townsend snow model" |
||||||||||||||||||||||||||
temp_air, poa_global, slant_height, lower_edge_drop_height, | ||||||||||||||||||||||||||
angle_of_repose=40): | ||||||||||||||||||||||||||
''' | ||||||||||||||||||||||||||
Calculates monthly snow loss based on a generalized monthly snow loss model | ||||||||||||||||||||||||||
discussed in [1]_. | ||||||||||||||||||||||||||
Comment on lines
+225
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Calculates monthly snow loss based on the Townsend monthly snow loss model [1]_." or Townsend-Powers. |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Parameters | ||||||||||||||||||||||||||
---------- | ||||||||||||||||||||||||||
snow_total : numeric | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our current definition of |
||||||||||||||||||||||||||
Inches of snow received in the current month. Referred as S in the | ||||||||||||||||||||||||||
paper [in] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
snow_events : numeric | ||||||||||||||||||||||||||
Number of snowfall events with snowfall > 1". Referred as N in the | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope - it isn't, hence removing it. I guess it is better to leave that up to the user (to define what a snow event is). |
||||||||||||||||||||||||||
paper [-] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
surface_tilt : numeric | ||||||||||||||||||||||||||
Array surface_tilt [deg] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
relative_humidity : numeric | ||||||||||||||||||||||||||
Relative humidity [%] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
temp_air : numeric | ||||||||||||||||||||||||||
Ambient temperature [°C] | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's worth explicitly using the word "monthly" for all the monthly inputs, and "average/total/etc" as appropriate. Temperature is monthly average, and I'm not seeing it in the reference but I guess RH is monthly average as well? |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
poa_global : numeric | ||||||||||||||||||||||||||
Plane of array insolation [kWh/m2/month] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
slant_height : float | ||||||||||||||||||||||||||
Row length in the slanted plane of array dimension [in] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
lower_edge_drop_height : float | ||||||||||||||||||||||||||
Drop height from array edge to ground [in] | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To discuss: what units should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer metric system over everything else :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other than a general reluctance to introduce imperial units to pvlib, the main reason I suggested cm is because snow.fully_covered_nrel takes snowfall in cm. Do non-US datasets use inches? I've only ever used snowfall data from the GHCN, which I think reports in inches. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm OK asking users to do some unit conversions to keep consistent units for similar functions in pvlib There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NOAA NCEI provides monthly climate normals in both metric and imperial units. I agree that sticking with metric units in pvlib is preferable. I think sticking with a single unit system within a single function is a must. |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
P : float | ||||||||||||||||||||||||||
kandersolar marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||||||||||||||||||||||||||
piled snow angle, assumed to stabilize at 40° , the midpoint of | ||||||||||||||||||||||||||
25°-55° avalanching slope angles [deg] | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Returns | ||||||||||||||||||||||||||
------- | ||||||||||||||||||||||||||
loss : numeric | ||||||||||||||||||||||||||
Average monthly DC capacity loss due to snow coverage [%] | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add a |
||||||||||||||||||||||||||
References | ||||||||||||||||||||||||||
---------- | ||||||||||||||||||||||||||
.. [1] Townsend, Tim & Powers, Loren. (2011). Photovoltaics and snow: An | ||||||||||||||||||||||||||
update from two winters of measurements in the SIERRA. Conference | ||||||||||||||||||||||||||
Record of the IEEE Photovoltaic Specialists Conference. | ||||||||||||||||||||||||||
003231-003236. 10.1109/PVSC.2011.6186627. | ||||||||||||||||||||||||||
Available at https://www.researchgate.net/publication/261042016_Photovoltaics_and_snow_An_update_from_two_winters_of_measurements_in_the_SIERRA | ||||||||||||||||||||||||||
''' # noqa: E501 | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
C1 = 5.7e04 | ||||||||||||||||||||||||||
C2 = 0.51 | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
snow_total_prev = np.roll(snow_total, 1) | ||||||||||||||||||||||||||
snow_events_prev = np.roll(snow_events, 1) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Se = _townsend_Se(snow_total, snow_events) | ||||||||||||||||||||||||||
Se_prev = _townsend_Se(snow_total_prev, snow_events_prev) | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Se_weighted = 1/3 * Se_prev + 2/3 * Se | ||||||||||||||||||||||||||
Comment on lines
+280
to
+283
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
gamma = (slant_height * Se_weighted * cosd(surface_tilt)) / \ | ||||||||||||||||||||||||||
(np.clip((lower_edge_drop_height**2 - Se_weighted**2), a_min=0.01, | ||||||||||||||||||||||||||
a_max=None) / 2 / tand(angle_of_repose)) | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't disagree with the spirit of having a lower bound of 0.01, but is there anything special about that value other than being small but nonzero? May be worth a comment explaining it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I was more wondering why specifically 0.01 is the lower limit (why not 0.1, 0.001, or any other small nonzero value?). I guess I was more curious than anything else -- the specific lower bound, so long as it's <<1, doesn't seem to matter much. Feel free to ignore this comment :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A common small value is eps or sometimes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, it seems the danger is having negative There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is pretty difficult for me to parse - can we break it up in a couple of assignments or be more aggressive with line breaks? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If I understand correctly... I mostly agree, but why can't gamma be 0? Shouldn't that be expected when snow fall is 0? I think mathematically gamma will blow up when both the snow fall is 0 and the drop height goes to 0. But I think in practice python will evaluate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+284
to
+286
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
GIT = 1 - C2 * np.exp(-gamma) | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also change this to |
||||||||||||||||||||||||||
loss = (C1 * Se_weighted * (cosd(surface_tilt))**2 * GIT * | ||||||||||||||||||||||||||
relative_humidity / (temp_air+273.15)**2 / poa_global**0.67) / 100 | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch that temperature is in Kelvin 👍 |
||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return loss |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,3 +95,32 @@ def test_dc_loss_nrel(): | |
expected = pd.Series([1, 1, .5, .625, .25, .5, 0]) | ||
actual = snow.dc_loss_nrel(snow_coverage, num_strings) | ||
assert_series_equal(expected, actual) | ||
|
||
|
||
def test__townsend_Se(): | ||
S = np.array([10, 10, 5, 1, 0, 0, 0, 0, 0, 0, 5, 10]) | ||
N = np.array([2, 2, 1, 0, 0, 0, 0, 0, 0, 0, 2, 3]) | ||
expected = np.array([7.5, 7.5, 5, 0, 0, 0, 0, 0, 0, 0, 3.75, 6.66666667]) | ||
actual = snow._townsend_Se(S, N) | ||
np.testing.assert_allclose(expected, actual, rtol=1e-07) | ||
|
||
|
||
def test_loss_townsend(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question for @cwhanse: referring to #1393 (comment), should we have a policy of always testing both array and Series for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think so There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say it's good practice but importance and extent depends on the model and implementation. I'm usually more concerned that we test for compatibility with scalars in "float, array, series" situations because I think it's easier for regressions to slip in with scalars (e.g. by introducing masking). |
||
snow_total = np.array([10, 10, 5, 1, 0, 0, 0, 0, 0, 0, 5, 10]) | ||
snow_events = np.array([2, 2, 1, 0, 0, 0, 0, 0, 0, 0, 2, 3]) | ||
surface_tilt = 20 | ||
relative_humidity = np.array([80, 80, 80, 80, 80, 80, 80, 80, 80, 80, | ||
80, 80]) | ||
temp_air = np.array([0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]) | ||
poa_global = np.array([350, 350, 350, 350, 350, 350, 350, 350, 350, 350, | ||
350, 350]) | ||
angle_of_repose = 40 | ||
slant_height = 100 | ||
lower_edge_drop_height = 10 | ||
expected = np.array([0.07696253, 0.07992262, 0.06216201, 0.01715392, 0, 0, | ||
0, 0, 0, 0, 0.02643821, 0.06068194]) | ||
actual = snow.loss_townsend(snow_total, snow_events, surface_tilt, | ||
relative_humidity, temp_air, | ||
poa_global, slant_height, | ||
lower_edge_drop_height, angle_of_repose) | ||
np.testing.assert_allclose(expected, actual, rtol=1e-05) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.