Skip to content
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

Negative "POA horizontal ratio" when sun behind array #641

Closed
adriesse opened this issue Jan 11, 2019 · 7 comments
Closed

Negative "POA horizontal ratio" when sun behind array #641

adriesse opened this issue Jan 11, 2019 · 7 comments

Comments

@adriesse
Copy link
Member

Describe the bug
The function poa_horizontal_ratio gives negative values when the sun is behind the array.

To Reproduce
pvlib.irradiance.poa_horizontal_ratio(60, 90, 60, 270)
-0.99999999999999933

Expected behavior
pvlib.irradiance.poa_horizontal_ratio(60, 90, 60, 270)
0.0

Versions:

  • pvlib.__version__: '0.5.2'
  • pandas.__version__: '0.19.2'
  • python: 3.5.1

Additional context
Similar problems were fixed in other irradiance functions already (#526).

@wholmgren
Copy link
Member

I think we considered this behavior in #535 and decided against limits in this function. 9a74404

@adriesse
Copy link
Member Author

adriesse commented Jan 12, 2019

Sorry, I realize I ran the example on an older version, but the new version looked the same here:

https://pvlib-python.readthedocs.io/en/latest/_modules/pvlib/irradiance.html

I guess that address currently is not showing the latest release?

@wholmgren
Copy link
Member

I believe that link shows the latest GitHub master. #535 went into 0.6.0, so I think release vs. master does not matter in this case.

I might not have been clear before. My initial sky diffuse bug-fix proposal included limits in poa_horizontal_ratio. That's shown in commit 9a74404. Based on reviewer feedback (in particular this comment from @cwhanse), I ultimately left poa_horizontal_ratio unchanged and instead applied limits within each diffuse function. That's shown in final #535 diff.

There may be applications where the negative value is useful (filtering data, bifacial), so I'm not convinced that this is really a bug. If we keep the behavior we could add a sentence to the documentation explaining the output when the sun is behind the panel.

@adriesse
Copy link
Member Author

Ok, I'll just close this then.

@wholmgren
Copy link
Member

@adriesse would you suggest that we change the behavior or at least the documentation? If so, happy to keep the issue open.

@adriesse adriesse reopened this Jan 14, 2019
@adriesse
Copy link
Member Author

adriesse commented Jan 14, 2019

Slightly changing both would be best in my view. Allowing aoi_projection to go negative is ok, but the docstring could be expanded to explain the meaning of negative values. I think that poa_horizontal_ratio should only use non-negative values of aoi_projection like the other users of that function. But I wouldn't put a high priority on it--I just noticed it as I was browsing the file looking for something else.

@kandersolar
Copy link
Member

poa_horizonal_ratio was removed in #2021. Shall we close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants