Snow coverage model from SAM

[JPalakapillyKWH]

• Closes Implement another snow loss model like Bill Marion as an alt to pvwatts #577
• I am familiar with the contributing guidelines.
• Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests (usually) must pass on the TravisCI and Appveyor testing services.
• Updates entries to docs/sphinx/source/api.rst for API changes.
• Adds description and name entries in the appropriate docs/sphinx/source/whatsnew file for all changes.
• Code quality and style is sufficient. Passes LGTM and SticklerCI checks.
• New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
• Pull request is nearly complete and ready for detailed review.

Brief description of the problem and proposed solution (if not already fully described in the issue linked to above):

I implemented the snow coverage model first proposed by Marion et al. in [1] and later validated and implemented in SAM [2]. Thought it might be useful to include in pvlib.

[1] Marion, B.; Schaefer, R.; Caine, H.; Sanchez, G. (2013). “Measured and modeled photovoltaic
system energy losses from snow for Colorado and Wisconsin locations.” Solar Energy 97; pp.
112-121.
[2] https://www.nrel.gov/docs/fy17osti/68705.pdf

[mikofski]

Yes! Thank you! Can you please search the issues for "snow" or "Marion" to find the ticket number and then add a comment that says, "closes #XYZ" thanks!

[JPalakapillyKWH]

Yes! Thank you! Can you please search the issues for "snow" or "Martin" to find the ticket number and then add a comment that says, "closes #XYZ" thanks!

Edited above comment.

[cwhanse]

I'm not opposed to a python native implementation of this model, but I'll ask a due diligence question: is it feasible, and desirable, to wrap the appropriate function in SAM SSC using nrel-pysam? I haven't located the snow model function in the SAM SSC so I don't know if 1) it does what we want, or 2) it is exposed by nrel-pysam. @janinefreeman

[janinefreeman]

@cwhanse The snow model implementation in SAM is exposed in nrel-pysam via cmod_snowmodel (https://github.com/NREL/ssc/blob/develop/ssc/cmod_snowmodel.cpp) which calls a shared library (https://github.com/NREL/ssc/blob/develop/shared/lib_snowmodel.cpp) that allows it to be integrated on the DC side in the detailed PV model.

[wholmgren]

I think there's value in a python implementation (particularly for people that want to play with the source code), I think there's value in pysam exposing the c implementation to python users (consistency with SAM, maybe speed), but what's the point of pvlib wrapping the pysam wrapper?

[JPalakapillyKWH]

I think there's value in a python implementation (particularly for people that want to play with the source code), I think there's value in pysam exposing the c implementation to python users (consistency with SAM, maybe speed), but what's the point of pvlib wrapping the pysam implementation?

Agree with this. I was able to fit data better by tuning some of the empirically determined coefficients in Marion's model.

[cwhanse]

I think there's value in a python implementation (particularly for people that want to play with the source code), I think there's value in pysam exposing the c implementation to python users (consistency with SAM, maybe speed), but what's the point of pvlib wrapping the pysam wrapper?

It is not easy (at least for me it isn't) to figure out how to use pysam to access specific SAM functions, so in cases where we are leveraging SAM functions, I think it is worth wrapping the wrapper for pvlib users' convenience.
In this case I think we should implement the snow model native in python for pvlib. It does not appear to me that pysam exposes the model parameters.

[janinefreeman]

That is true, the SAM implementation doesn't expose the model parameters because most of our users do not have the data necessary to tweak those. On a related note, @JPalakapillyKWH if any of your analysis tuning the parameters to the model is publish-able, the industry would definitely appreciate that :)

[cwhanse]

@wholmgren do we want this PR as it's own module, or bundled with other code into a more general "irradiance_loss" module? I'm inclined to the former, anticipating a "soiling" module, whatever we call the "iam" module, "shading" module, etc.

[wholmgren]

I also favor putting this in its own module. Maybe snow instead snowcoverage? Not sure coverage adds anything and Anton recently reminded us that module names should be brief.

We should probably think harder about if/when/how to interface with pysam elsewhere.

[cwhanse]

@JPalakapillyKWH thanks for this contribution! A partial review focused on consistency with the references, terms and interface with the rest of pvlib.

[kandersolar]

@cwhanse I like this rewrite and how it declutters the namespace. Is it worth mentioning in the docstring that this implementation differs from the original in that the original works in "tenths of row slant height"?

[cwhanse]

Test failures are for pvlib.bifacial.pvfactors. I believe this PR is ready.

[kandersolar]

Some small doc suggestions. Also, consider adding @JPalakapillyKWH to the contributors list for getting the ball rolling.

[kandersolar]

I included a comment in that review but I think it got buried back in the thread history, so I'll repost it -- I think this suggestion didn't make it into the PR: #764 (comment)

[CameronTStark]

Looks great! I appreciate your commitment on this @cwhanse.

I'll leave it open for a little while longer for others to review, if desired, then I'll merge.

Thank you!