Skip to content

Conversation

@jsstein
Copy link
Contributor

@jsstein jsstein commented Jan 29, 2021

  • Closes pvlib.irradiance.reindl() model generates NaNs when GHI = 0 #1153
  • I am familiar with the contributing guidelines
  • Tests updated
    - [ ] Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

Added error trapping/exception handling to result in sky diffuse = 0 when GHI=0. Revised the test problem.

@cwhanse cwhanse added the bug label Jan 29, 2021
@cwhanse cwhanse added this to the 0.9.0 milestone Jan 29, 2021
result = irradiance.reindl(
40, 180, irrad_data['dhi'], irrad_data['dni'], irrad_data['ghi'],
dni_et, ephem_data['apparent_zenith'], ephem_data['azimuth'])
# values from matlab 1.4 code
Copy link
Member

Choose a reason for hiding this comment

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

does this comment need to be updated?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's OK, can't have been meant literally since Matlab doesn't produce np.nan.

(:issue:`1065`, :pull:`1140`)

* Reindl model fixed to generate sky_diffuse=0 when GHI=0.
(:issue:'1153', :pull:'1154', :ghuser:'jsstein')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(:issue:'1153', :pull:'1154', :ghuser:'jsstein')
(:issue:`1153`, :pull:`1154`, :ghuser:`jsstein`)

Copy link
Member

Choose a reason for hiding this comment

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

I was about to merge when I noticed these single quotes need to be back ticks.

Traditionally we've not put contributor names next to individual lines in the what's new file, but I have no objection to it. I guess I'd still put the contributor name at the end, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed a formatting correction and removed the contributor info. Thanks for the review!

@wholmgren wholmgren merged commit 975b798 into pvlib:master Feb 2, 2021
@wholmgren
Copy link
Member

Thanks @jsstein!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pvlib.irradiance.reindl() model generates NaNs when GHI = 0

3 participants