-
Notifications
You must be signed in to change notification settings - Fork 1k
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
port Purdue bifacial irradiance model #914
Conversation
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.
Test failure is for unrelated remote data
@nappaillav are you planning on adding the bifacial code to this PR or did you want to split it into two PRs? My preference is a single PR since the trig functions are not explicitly tested and I don't see another near-term use for them. |
@wholmgren Initially I thought of having separate pull requests, but I guess its better to combine both as you suggest. |
There are few more steps in the process of completed the port of Purdue bifacial irradiance model, which are as follows
Currently I'm updating the code for your perusal. So that you can suggest me |
@cwhanse I have updated the description. If you can let me know if the implementation is fine by you, we can proceed further. |
The new bifacial functions should be added to I'd suggest Although I'm not fond of the
Sure. Before we do that, we need to clean up the commit - it's not reviewable in current form. There's a lot of leftover Matlab formalities that need to be dropped or formatted to pass Stickler. There's a bunch of recent commits to pvlib master in other PRs that show up here as new code changes @wholmgren should this be addressed with a git rebase? |
Yes |
I will rebase and make sure the commit is clean |
Closing due to merge of #717 |
docs/sphinx/source/api.rst
for API changes.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`
).This is related to issue #863