Create plot_ghi_transposition.py example #933

ericf900 · 2020-03-11T00:18:57Z

I am familiar with the contributing guidelines
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.

Example showing transposition from GHi to POA, and comparing winter transposition gain to summer.

kandersolar · 2020-03-11T00:39:11Z

Hey @ericf900, cool example! This PR's version of the docs are built here, so you can check out how your example looks when it's live (the build takes a few minutes after you push an update): https://pvlib-python--933.org.readthedocs.build/en/933/

A couple notes:

The docs build system only build files whose names start with "plot_", that's why your example isn't showing up like the others. If you rename the file and then push that commit, the docs will get rebuilt.
Typically we strip out the spyder header (# -*- coding: utf-8 -*- / Created on... etc) before merging into pvlib
You should add an entry about this example under the Documentation header and (if you're comfortable with it) your name to the Contributors section in the version 0.7.2 whatsnew file: https://github.com/pvlib/pvlib-python/blob/master/docs/sphinx/source/whatsnew/v0.7.2.rst

kandersolar · 2020-03-11T00:42:06Z

Also can you merge upstream/master into your branch? Some fixes were recently made to clean up the documentation build log and it would make reviewing this a little easier.

…mple

…vlib-python into Transposition-example

ericf900 · 2020-03-11T02:07:18Z

@kanderso-nrel Thanks for the suggestions! I made those updates and updated the PR

kandersolar

Here are some suggestions. The :py:meth:... stuff will auto-link the functions to their documentation entries (see here for an example of what it looks like). Otherwise looks good to me!

docs/examples/plot_ghi_transposition.py

kandersolar · 2020-03-11T02:21:14Z

docs/examples/plot_ghi_transposition.py

+
+# For this example, we will be using Golden, Colorado
+tz = 'MST'
+lat, lon = 39.755, -105.221


very nice choice of location, +1

docs/examples/plot_ghi_transposition.py

kandersolar · 2020-03-11T02:27:35Z

docs/examples/plot_ghi_transposition.py

+        dni=clearsky_ghi['dni'],
+        ghi=clearsky_ghi['ghi'],
+        dhi=clearsky_ghi['dhi'],
+        solar_zenith=solar_position['zenith'],


Suggested change

solar_zenith=solar_position['zenith'],

solar_zenith=solar_position['apparent_zenith'],

FYI I'd like someone else to confirm that using apparent solar position is appropriate before making this change

According to the parameters for each transposition model option in get_total_irradiance, apparent_zenith should be passed through, rather than zenith.

It's a different matter to verify that each model actually expects apparent_zenith rather than zenith. I'll look at the underlying references when I can find the opportunity.

docs/examples/plot_ghi_transposition.py

Co-Authored-By: Kevin Anderson <[email protected]>

…vlib-python into Transposition-example

mikofski · 2020-03-11T17:39:34Z

docs/examples/plot_ghi_transposition.py

+winter_irradiance = get_irradiance(site, '12-21-2020', 25, 180)
+
+# Plot GHI vs. POA for winter and summer
+fig, (ax1, ax2) = plt.subplots(1, 2)


This example is really great! Thanks!

I have a minor suggestion, can you vertically stack the plots and share the x-axis?

fig, ax = plt.subplots(2, 1, sharex=True) # stack plots (2, 1) and share the x axis ax1, ax2 = ax # or you can just use ax[0] instead of ax1, and ax[1] instead ax2, minor preference

I'm having a hard time reading the dates on the x-axis because they're a bit crowded, so I thought trying them vertical and sharing them might look nicer, but your call - but not a blocker for me - also fine as is, THANKS!

Unfortunately the indexes are 6 months apart so I don't think sharex=True will look good. Could convert the datetime index to a nicer string form with df.index.strftime("%H:%M"). Or could drop sharex=True, but then the x-label for the upper axes might overlap the bottom axes.

Hi Mike and Kevin! I am looking at this now- I agree the x-axis is a bit cluttered. I think the benefit of keeping the plots side-by-side is that it highlights that, while there is not much of a gain for POA compared to GHI in the summer, overall irradiance is higher.

I'll make some edits now and push up the new version!

You're both right! Sorry for the distraction! Great work

cwhanse

Thanks @ericf900

cwhanse · 2020-03-11T15:27:56Z

docs/examples/plot_ghi_transposition.py

+plt.show()
+
+# %%
+# Note that in Summer, there is not much gain when comparing POA irradiance to


+1 for this note, answers a question often asked by newcomers to PV modeling

docs/examples/plot_ghi_transposition.py

cwhanse · 2020-03-11T19:18:16Z

docs/examples/plot_ghi_transposition.py

+        dni=clearsky_ghi['dni'],
+        ghi=clearsky_ghi['ghi'],
+        dhi=clearsky_ghi['dhi'],
+        solar_zenith=solar_position['zenith'],


According to the parameters for each transposition model option in get_total_irradiance, apparent_zenith should be passed through, rather than zenith.

It's a different matter to verify that each model actually expects apparent_zenith rather than zenith. I'll look at the underlying references when I can find the opportunity.

docs/examples/plot_ghi_transposition.py

mikofski

Eric, this is a really great contribution! Thanks so much. I really like the comments, super thorough and easy to read. Very good explanation of the process of generating clear-sky and transposing to POA. Awesome!

mikofski · 2020-03-11T23:03:43Z

docs/examples/plot_ghi_transposition.py

+winter_irradiance['POA'].plot(ax=ax2, label='POA')
+ax1.set_xlabel('Time of day (Summer)')
+ax2.set_xlabel('Time of day (Winter)')
+ax1.set_ylabel('Irradiance (W/m2)')


you can use latex here for W/m^2 if you want, it might look a little bit nicer?

ax1.set_ylabel('Irradiance ($W/m^2$)')

I also added gridlines, but I noticed other examples don't have them, so just my personal preference, dk what the convention is.

Thanks for the feedback Mike! Made these changes and am uploading a new version now :)

wholmgren

Thanks @ericf900 for the great example and @kanderso-nrel @mikofski @cwhanse for the reviews. I suggest merging as is.

ericf900 added 3 commits March 10, 2020 20:12

Create ghi_transposition.py

d3eb73f

Update ghi_transposition.py

c8f8fa0

Update ghi_transposition.py

108f5b8

ericf900 added 5 commits March 10, 2020 20:49

Create plot_ghi_transposition.py

8ea6149

Delete ghi_transposition.py

f04591e

Merge remote-tracking branch 'upstream/master' into Transposition-exa…

f94f8d7

…mple

Update v0.7.2.rst

cea8d2d

Merge branch 'Transposition-example' of https://github.com/ericf900/p…

dc0950a

…vlib-python into Transposition-example

Update v0.7.2.rst

d7d84f2

kandersolar reviewed Mar 11, 2020

View reviewed changes

ericf900 and others added 9 commits March 11, 2020 09:27

Update docs/examples/plot_ghi_transposition.py

9ff04b2

Co-Authored-By: Kevin Anderson <[email protected]>

Update docs/examples/plot_ghi_transposition.py

8308bc6

Co-Authored-By: Kevin Anderson <[email protected]>

Update docs/examples/plot_ghi_transposition.py

6763338

Co-Authored-By: Kevin Anderson <[email protected]>

Update plot_ghi_transposition.py

4f1ef52

Update docs/examples/plot_ghi_transposition.py

e806e29

Co-Authored-By: Kevin Anderson <[email protected]>

Update docs/examples/plot_ghi_transposition.py

8bc57d3

Co-Authored-By: Kevin Anderson <[email protected]>

Update plot_ghi_transposition.py

7fefc74

Merge branch 'Transposition-example' of https://github.com/ericf900/p…

e23a6d3

…vlib-python into Transposition-example

Update plot_ghi_transposition.py

a2446dd

mikofski reviewed Mar 11, 2020

View reviewed changes

cwhanse approved these changes Mar 11, 2020

View reviewed changes

wholmgren added this to the 0.7.2 milestone Mar 11, 2020

wholmgren added the documentation label Mar 11, 2020

Update plot_ghi_transposition.py

86a1970

mikofski reviewed Mar 11, 2020

View reviewed changes

docs/examples/plot_ghi_transposition.py Outdated Show resolved Hide resolved

mikofski reviewed Mar 11, 2020

View reviewed changes

docs/examples/plot_ghi_transposition.py Outdated Show resolved Hide resolved

mikofski reviewed Mar 11, 2020

View reviewed changes

docs/examples/plot_ghi_transposition.py Outdated Show resolved Hide resolved

mikofski approved these changes Mar 11, 2020

View reviewed changes

mikofski reviewed Mar 11, 2020

View reviewed changes

Update plot_ghi_transposition.py

0345815

wholmgren approved these changes Mar 12, 2020

View reviewed changes

wholmgren changed the title ~~Create ghi_transposition.py~~ Create plot_ghi_transposition.py example Mar 17, 2020

wholmgren merged commit e526b55 into pvlib:master Mar 17, 2020

kandersolar mentioned this pull request Jun 20, 2020

Add hsu soiling model gallery example. #990

Merged

6 tasks

	solar_zenith=solar_position['zenith'],
	solar_zenith=solar_position['apparent_zenith'],

Create plot_ghi_transposition.py example #933

Create plot_ghi_transposition.py example #933

Uh oh!

Conversation

ericf900 commented Mar 11, 2020 • edited by wholmgren Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kandersolar commented Mar 11, 2020

Uh oh!

kandersolar commented Mar 11, 2020

Uh oh!

ericf900 commented Mar 11, 2020

Uh oh!

kandersolar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mikofski Mar 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cwhanse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikofski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wholmgren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ericf900 commented Mar 11, 2020 •

edited by wholmgren

Loading

mikofski Mar 11, 2020 •

edited

Loading