Correct condition 5 in clearsky.detect_clearsky

[cwhanse]

Describe the bug
Condition 5 in clearsky.detect_clearsky does not agree with Eq. 12 - 14 of the reference. See #505

Expected behavior
clearsky.detect_clearsky currently calculates condition 5 as the difference in maximum absolute slopes, rather than the maximum of the absolute differences in slopes.

Versions:

• pvlib.__version__: 0.5

[wholmgren]

If I understand correctly, the pvlib code is essentially:

X  = max(abs(meas_slope)) - max(abs(clear_slope))
c5 = X < slope_dev

but Reno and Hansen describe:

X  = max(abs(meas_slope - clear_slope))
   = max(abs(line_diff_slope))
c5 = X < slope_dev

It would be good to include an example/test where this changes the overall result.

[cwhanse]

@wholmgren you understand correctly.

Do we need to test before/after when fixing a bug though?

[adriesse]

Perhaps not in general, but in this case it sound like the published results may have been based on code with a bug, so technically you should perhaps run the corrected code, check the results and publish an erratum.

[wholmgren]

@cwhanse my feeling is that the detect_clearsky_data.csv test data should include values that produce a different test result when the code is changed.

[cwhanse]

Of course, I agree that we should correct the test along with the code fix. I can publish a comparison on pvpmc.sandia.gov, or post the results here.