-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix omission in snow_coverage_nrel #2292
base: main
Are you sure you want to change the base?
Changes from 2 commits
3594450
9716bc8
cd579f2
c11aee4
7194e3c
11cf12e
b6424e3
d46837c
0ef3d7a
57f5eef
81c44d7
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 | ||||
---|---|---|---|---|---|---|
|
@@ -13,30 +13,36 @@ def _time_delta_in_hours(times): | |||||
return delta.dt.total_seconds().div(3600) | ||||||
|
||||||
|
||||||
def fully_covered_nrel(snowfall, threshold_snowfall=1.): | ||||||
def fully_covered_nrel(snowfall, snow_depth=None, threshold_snowfall=1.): | ||||||
''' | ||||||
Calculates the timesteps when the row's slant height is fully covered | ||||||
by snow. | ||||||
|
||||||
Parameters | ||||||
---------- | ||||||
snowfall : Series | ||||||
Accumulated snowfall in each time period [cm] | ||||||
|
||||||
threshold_snowfall : float, default 1.0 | ||||||
snowfall: Series | ||||||
Accumulated snowfall in each time period. [cm] | ||||||
snow_depth: Series, optional | ||||||
Snow depth on the ground at the beginning of each time period. | ||||||
Must have the same index as `snowfall`. [cm] | ||||||
threshold_snowfall: float, default 1.0 | ||||||
Hourly snowfall above which snow coverage is set to the row's slant | ||||||
height. [cm/hr] | ||||||
|
||||||
Returns | ||||||
---------- | ||||||
boolean: Series | ||||||
Series | ||||||
True where the snowfall exceeds the defined threshold to fully cover | ||||||
the panel. | ||||||
|
||||||
Notes | ||||||
----- | ||||||
Implements the model described in [1]_ with minor improvements in [2]_. | ||||||
|
||||||
`snow_depth` is used to set coverage=0 when no snow is present on the | ||||||
ground. This check is described in [2]_ as needed for systems with | ||||||
low tilt angle. | ||||||
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 this explanation fits better the |
||||||
|
||||||
References | ||||||
---------- | ||||||
.. [1] Marion, B.; Schaefer, R.; Caine, H.; Sanchez, G. (2013). | ||||||
|
@@ -56,11 +62,15 @@ def fully_covered_nrel(snowfall, threshold_snowfall=1.): | |||||
hourly_snow_rate.iloc[0] = snowfall.iloc[0] / timedelta | ||||||
else: # can't infer frequency from index | ||||||
hourly_snow_rate.iloc[0] = 0 # replaces NaN | ||||||
return hourly_snow_rate > threshold_snowfall | ||||||
covered = (hourly_snow_rate > threshold_snowfall) | ||||||
# no coverage when no snow on the ground | ||||||
if snow_depth is not None: | ||||||
covered = covered & (snow_depth > 0.) | ||||||
return covered | ||||||
|
||||||
|
||||||
def coverage_nrel(snowfall, poa_irradiance, temp_air, surface_tilt, | ||||||
initial_coverage=0, threshold_snowfall=1., | ||||||
snow_depth=None, initial_coverage=0, threshold_snowfall=1., | ||||||
can_slide_coefficient=-80., slide_amount_coefficient=0.197): | ||||||
''' | ||||||
Calculates the fraction of the slant height of a row of modules covered by | ||||||
|
@@ -82,6 +92,9 @@ def coverage_nrel(snowfall, poa_irradiance, temp_air, surface_tilt, | |||||
surface_tilt : numeric | ||||||
Tilt of module's from horizontal, e.g. surface facing up = 0, | ||||||
surface facing horizon = 90. [degrees] | ||||||
snow_depth : Series, optional | ||||||
Snow depth on the ground at the beginning of each time period. | ||||||
Must have the same index as `snowfall`. [cm] | ||||||
initial_coverage : float, default 0 | ||||||
Fraction of row's slant height that is covered with snow at the | ||||||
beginning of the simulation. [unitless] | ||||||
|
@@ -106,6 +119,10 @@ def coverage_nrel(snowfall, poa_irradiance, temp_air, surface_tilt, | |||||
In [1]_, `can_slide_coefficient` is termed `m`, and the value of | ||||||
`slide_amount_coefficient` is given in tenths of a module's slant height. | ||||||
|
||||||
`snow_depth` is used to set coverage=0 when no snow is present on 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.
Suggested change
|
||||||
ground. This check is described in [2]_ as needed for systems with | ||||||
low tilt angle. | ||||||
|
||||||
References | ||||||
---------- | ||||||
.. [1] Marion, B.; Schaefer, R.; Caine, H.; Sanchez, G. (2013). | ||||||
|
@@ -117,7 +134,7 @@ def coverage_nrel(snowfall, poa_irradiance, temp_air, surface_tilt, | |||||
''' | ||||||
|
||||||
# find times with new snowfall | ||||||
new_snowfall = fully_covered_nrel(snowfall, threshold_snowfall) | ||||||
new_snowfall = fully_covered_nrel(snowfall, snow_depth, threshold_snowfall) | ||||||
|
||||||
# set up output Series | ||||||
snow_coverage = pd.Series(np.nan, index=poa_irradiance.index) | ||||||
|
@@ -143,6 +160,10 @@ def coverage_nrel(snowfall, poa_irradiance, temp_air, surface_tilt, | |||||
snow_coverage.ffill(inplace=True) | ||||||
snow_coverage -= cumulative_sliding | ||||||
|
||||||
if snow_depth is not None: | ||||||
# no coverage when there's no snow on the ground | ||||||
# described in [2] to avoid non-sliding snow for low-tilt systems. | ||||||
snow_coverage[snow_depth <= 0] = 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. I don't think this check can be applied at the end like this. In cases where the check takes effect (snow_depth = 0 --> coverage = 0), that effect should propagate forward in time and influence values for future timestamps. As it is now, I don't think this code allows that. See for example this input: times = pd.date_range("2019-01-01", freq="h", periods=4)
snowfall = pd.Series([10, 0, 0, 0.1], index=times) # last value is below threshold_snowfall
snow_depth = pd.Series([10, 5, 0, 0.1], index=times)
poa_irradiance = pd.Series(100, index=times)
temp_air = pd.Series(-1, index=times)
surface_tilt = 10
coverage_nrel(snowfall, poa_irradiance, temp_air, surface_tilt, snow_depth)
# output:
2019-01-01 00:00:00 1.000000
2019-01-01 01:00:00 0.965791
2019-01-01 02:00:00 0.000000
2019-01-01 03:00:00 0.897374 # this value doesn't make sense, should be zero 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 you are correct. The SAM implementation appears to iterate over timesteps. 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 the SAM algorithm, these lines set coverage to full if both of two conditions are met:
So change in snow depth exceeds deltaThreshold (*dt converts to hourly), and snowDepth exceeds a different threshold so the snow is "sticking" around. |
||||||
# clean up periods where row is completely uncovered | ||||||
return snow_coverage.clip(lower=0) | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,18 @@ def test_fully_covered_nrel(): | |
assert_series_equal(expected, fully_covered) | ||
|
||
|
||
def test_fully_covered_nrel_with_snow_depth(): | ||
dt = pd.date_range(start="2019-1-1 12:00:00", end="2019-1-1 18:00:00", | ||
freq='1h') | ||
snowfall_data = pd.Series([1, 5, .6, 4, .23, -5, 19], index=dt) | ||
snow_depth = pd.Series([0., 1, 6, 6.6, 10.6, 10., -2], index=dt) | ||
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 guess if a negative snow_depth is given, it is handled as 0, right? Maybe there should be a warning about this. Or should an error be raised instead? 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 in general pvlib doesn't do quality checks of user input data unless bad data affects the output of the function. |
||
expected = pd.Series([False, True, False, True, False, False, False], | ||
index=dt) | ||
fully_covered = snow.fully_covered_nrel(snowfall_data, | ||
snow_depth=snow_depth) | ||
assert_series_equal(expected, fully_covered) | ||
|
||
|
||
def test_coverage_nrel_hourly(): | ||
surface_tilt = 45 | ||
slide_amount_coefficient = 0.197 | ||
|
@@ -38,6 +50,27 @@ def test_coverage_nrel_hourly(): | |
assert_series_equal(expected, snow_coverage) | ||
|
||
|
||
def test_coverage_nrel_hourly_with_snow_depth(): | ||
surface_tilt = 45 | ||
slide_amount_coefficient = 0.197 | ||
dt = pd.date_range(start="2019-1-1 10:00:00", end="2019-1-1 17:00:00", | ||
freq='1h') | ||
poa_irradiance = pd.Series([400, 200, 100, 1234, 134, 982, 100, 100], | ||
index=dt) | ||
temp_air = pd.Series([10, 2, 10, 1234, 34, 982, 10, 10], index=dt) | ||
snowfall_data = pd.Series([1, .5, .6, .4, .23, -5, .1, .1], index=dt) | ||
snow_depth = pd.Series([1, 1, 1, 1, 0, 1, 0, .1], index=dt) | ||
snow_coverage = snow.coverage_nrel( | ||
snowfall_data, poa_irradiance, temp_air, surface_tilt, | ||
snow_depth=snow_depth, threshold_snowfall=0.6) | ||
|
||
slide_amt = slide_amount_coefficient * sind(surface_tilt) | ||
covered = 1.0 - slide_amt * np.array([0, 1, 2, 3, 4, 5, 6, 7]) | ||
expected = pd.Series(covered, index=dt) | ||
expected[snow_depth <= 0] = 0 | ||
assert_series_equal(expected, snow_coverage) | ||
|
||
|
||
def test_coverage_nrel_subhourly(): | ||
surface_tilt = 45 | ||
slide_amount_coefficient = 0.197 | ||
|
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.
Should we have a name for the returned parameter? E.g., snow_coverage?