-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Reindl model bug fix to close #1153 #1154
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
Conversation
| 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| (:issue:'1153', :pull:'1154', :ghuser:'jsstein') | |
| (:issue:`1153`, :pull:`1154`, :ghuser:`jsstein`) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
|
Thanks @jsstein! |
- [ ] Updates entries todocs/sphinx/source/api.rstfor API changes.docs/sphinx/source/whatsnewfor 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`).Added error trapping/exception handling to result in sky diffuse = 0 when GHI=0. Revised the test problem.