Skip to content

Conversation

@adriesse
Copy link
Member

@adriesse adriesse commented Jan 19, 2019

  • Enhances the solution for limit cos(aoi) in diffuse sky functions #535
  • 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.

Calculates the dot product of the solar vector and the surface
normal.
Calculates the dot product of the sun position unit vector and the surface
normal unit vector; in other words, the cosine of the angle of incidence.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to improve this even slightly more, can you use LaTeX to add the equation?

$$AOI_{projection} = \\cos{ \\beta } \\cos{ \\left( \\text{ solar zenith } \\right) } + \\sin{ \\beta } \\sin{ \\left( \\text{ solar zenith } \\right) } \\cos{ \\left( \\text{ solar azimuth} \\right) - \\left( \\text{ surface azimuth} \\right) }$$

might require some creative thinking about symbols, and some text here or elsewhere to define them. Just an idea

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next time?

Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adriesse.

Ok with me to merge as is.

@cwhanse
Copy link
Member

cwhanse commented Jan 21, 2019

thanks @adriesse

@cwhanse cwhanse closed this Jan 21, 2019
@cwhanse cwhanse reopened this Jan 21, 2019
@cwhanse cwhanse merged commit 52eeae7 into pvlib:master Jan 21, 2019
@wholmgren wholmgren added this to the 0.6.1 milestone Jan 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants