Fix omission in snow_coverage_nrel

[cwhanse]

• Closes snow.coverage_nrel not fully implemented according to reference #1171
• I am familiar with the contributing guidelines
• Tests added
• Updates entries in docs/sphinx/source/reference for API changes.
• Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/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`).
• New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
• Pull request is nearly complete and ready for detailed review.
• Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

Additional scope if I can get clarity: describe whether snowfall is left- or right-aligned.

[IoannisSifnaios]

Some ideas from my side. Otherwise looks good to me!

[kandersolar]

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

[cwhanse]

I think you are correct. The SAM implementation appears to iterate over timesteps.

[cwhanse]

In the SAM algorithm, these lines set coverage to full if both of two conditions are met:

	if ((snowDepth - previousDepth) >= deltaThreshold*dt && snowDepth >= depthThreshold){
		coverage = 1;
	}

So change in snow depth exceeds deltaThreshold (*dt converts to hourly), and snowDepth exceeds a different threshold so the snow is "sticking" around.

[cwhanse]

@kandersolar I think I've corrected the implementation of the check

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)

Out[3]: 
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.000000
Freq: H, dtype: float64

[kandersolar]

The new approach makes sense to me. A few minor comments, and there's a merge conflict.

[kandersolar]

Is this comment regarding the value in [2] correct? In the last paragraph on page 3 ("Implementation in SAM"), I see:

We set these thresholds equal to 1 cm for consistency with snow depth measurement uncertainties

[cwhanse]

I read that paragraph as describing the detection of new snow fall. I think the intent is clear, however - no snow on the panels if there's minimal snow on the ground.

[kandersolar]

Thanks @cwhanse