refactor sapm to be consistent with matlab

[wholmgren]

I updated to pvsystem.sapm to be consistent with the PVLIB MATLAB API. pvsystem.sapm now takes an effective irradiance argument instead of POA irradiances, airmass, and AOI. It also implements closely related sapm_spectral_loss, sapm_aoi_loss, and sapm_effective_irradiance functions, as well as PVSystem methods. See the changed files for details.

I need to write some tests for the new functions and methods. I also need to update ModelChain. It might break some documentation, too.

Let me know if you have any comments or concerns.

closed #198, closes #205

[wholmgren]

I think this is ready to go now. I'll merge it in the next few days unless I hear objections. It's probably worthwhile for you to look at the changes if you care about the SAPM model.

[cwhanse]

I agree with the changes to the SAPM interface. In the reflection loss function, I'd also impose an upper bound of 1 (there's already a lower bound of 0).

[wholmgren]

Thanks, Cliff. Added an upper bound to the reflection loss function.

[wholmgren]

I thought this was ready, but a new problem came up: Adding the upper limit changed some modelchain tests by about 0.5%.

Here's the 0-1 clipped and un-clipped sapm_aoi_loss results for our standard test module.

Indeed, the coefficients are greater than 1 from about 15 to 40 degrees. The maximum coefficient is 1.0078. The existing sapm function does not clip the internal aoi loss calculation, so adding the clip changed a few results.

We should probably implement the function in the same way that the data was originally fit since it's a purely empirical model.

Also, the function should either take the absolute value of aoi or return nan for aoi less than 0.

[cwhanse]

We should probably implement the function in the same way that the data was originally fit since it's a purely empirical model.

I disagree. The SAPM function fits a polynomial without a constraint that f2(AOI)<=1. It’s a bad choice for an empirical function. This is a sore point with me, because physically, you shouldn’t increase POA if you move away from normal (AOI=0), and by definition f2(0) = 1. We’ve revisited our outdoor measurement procedures and determined that the ‘measurements’ (in quotes because you compute values of f2(AOI) from measured Isc and irradiance) of f2(AOI) which slightly exceeded 1 were erroneous, for several reasons. Bruce King had a paper at PVSC 42(?) on this subject.

Also, the function should either take the absolute value of aoi or return nan for aoi less than 0.

I’d return nan and a warning if AOI<0, that’s a problem with the inverse cosine implementation. By definition you should get 0<=AOI<180. AOI>90 indicates that the sun in behind the panel.

Cliff

[wholmgren]

Thanks for the background on the measurement and fit procedures. I now agree that it should be clipped. I will make a note of the change in the whats new file.

[adriesse]

I fully agree that the polynomial function is a poor choice for the AOI characteristics. However I would recommend implementing it without clipping at 1.0 for these reasons:

• The original model doesn't have the clipping, so it would not be an accurate implementation of that model.
• If the original model had clipping, the fit coeffiencients would be different, so it would not be strictly correct to use the coefficients from one with the other.

On a related note, I have definitely measured values > 1.0 for some irradiance sensors. I think it could happen with modules as well, with certain surface textures. I always thought that values > 1.0 with the SAPM model were just an artifact of the polynomial fit, but am now curious to learn more about the actual measurements. (Incidentally, there are sensors that respond beyond 90 degrees as well.)

I am of course also in favour of improving various models, but they should be identified as improved models rather than take the place of existing ones, especially if the existing one have been around for a while.

Anton

[wholmgren]

@adriesse I don't understand your statement about measuring AOI loss coefficients > 1.0. Sure, you can measure values greater than 1.0 if there's sufficient statistical noise or systematic errors, but given the exceedingly strong prior information, I wouldn't accept that the true value is greater than 1.0.

I'm willing to add a clip_upper keyword argument to the function, but I'm not inclined to propagate that feature through the PVSystem and ModelChain APIs.

[adriesse]

@wholmgren I'll write a paper about the topic soon I hope, but my two objections are unrelated to whether or not there really might be values > 1.0: I just think it would be preferable to implement the historical model as it was published.

[wholmgren]

@adriesse I generally agree with your objections if we consider the model to be independent of the sapm parameter database. However, I've always thought of the model and database as a coupled system and given the measurement errors described by @cwhanse, I see the addition of an upper limit as fixing something that's broken. My perspective could certainly be too limited and I could be wrong.

@cwhanse are you still in favor of imposing an upper limit?

I believe that the paper that Cliff is referring to is "Recent Advancements in Outdoor Measurement Techniques for Angle of Incidence Effects" in IEEE PVSC 42.

[cwhanse]

That is the paper.

I understand Anton’s point of view, which is that the software should be consistent with the published model description. That’s an underlying motivation for PVLib. So, I’ll withdraw my suggestion. However, in the interest of providing better model results, I like your idea to add capability to limit model output to <=1, as an option.

Cliff

[wholmgren]

Ok. I added an "upper" keyword argument to the function. The default is no clipping. I also added the new reference.

I think we can merge now.