Skip to content

Return diffuse components from Perez transposition model #259

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Dec 2, 2016
Merged

Return diffuse components from Perez transposition model #259

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c0b3c05
Saving diffuse components into dict to be returned
Nov 1, 2016
a02930d
Wrote test for diffuse components
Nov 1, 2016
f59026d
Merge branch 'master' into perez_transp_components
Nov 8, 2016
ea3ea1d
Updated docstring and fixed test for Python3
Nov 17, 2016
bd3bee9
Merge branch 'master' into perez_transp_components
Nov 18, 2016
33a0141
Now testing each calculated component
Nov 18, 2016
fc5d6b1
Updated whatsnew file with changes
Nov 18, 2016
fb0c60a
Cleaning changes
Nov 18, 2016
29da785
Check that component col order is correct in test
Nov 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/sphinx/source/whatsnew/v0.4.2.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Enhancements
separate pages for each function, class, or method. (:issue:`258`)
* Improved Linke turbidity factor time interpolation with Python `calendar`
month days and leap years (:issue:`265`)
* Added option to return diffuse components from Perez transposition model.


Other
Expand All @@ -61,3 +62,4 @@ Code Contributors
* Will Holmgren
* Volker Beutner
* Mark Mikofski
* Marc Anoma
24 changes: 22 additions & 2 deletions pvlib/irradiance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ def king(surface_tilt, dhi, ghi, solar_zenith):

def perez(surface_tilt, surface_azimuth, dhi, dni, dni_extra,
solar_zenith, solar_azimuth, airmass,
model='allsitescomposite1990'):
model='allsitescomposite1990', return_components=False):
'''
Determine diffuse irradiance from the sky on a tilted surface using
one of the Perez models.
Expand Down Expand Up @@ -953,6 +953,10 @@ def perez(surface_tilt, surface_azimuth, dhi, dni, dni_extra,
* 'capecanaveral1988'
* 'albany1988'

return_components: bool (optional, default=False)
Flag used to decide whether to return the calculated diffuse components
or not.

Returns
--------
sky_diffuse : numeric
Expand Down Expand Up @@ -1043,7 +1047,23 @@ def perez(surface_tilt, surface_azimuth, dhi, dni, dni_extra,
else:
sky_diffuse = np.where(np.isnan(airmass), 0, sky_diffuse)

return sky_diffuse
if return_components:
component_keys = ('isotropic', 'circumsolar', 'horizon')
diffuse_components = dict.fromkeys(component_keys)
Copy link
Member

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but I don't think the fromkeys call is doing anything useful for us here. You can just call diffuse_components = OrderedDict() and delete the component_keys variable. The keys will created, in order, in the lines below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure (see following commit). I was just wondering why we wanted to use an ordered dictionary instead of a regular one here?


# Calculate the different components
diffuse_components['isotropic'] = dhi * term1
diffuse_components['circumsolar'] = dhi * term2
diffuse_components['horizon'] = dhi * term3

# Set values of components to 0 when sky_diffuse is 0
for k in diffuse_components.keys():
diffuse_components[k][sky_diffuse == 0] = 0
Copy link
Member

Choose a reason for hiding this comment

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

This line will fail with scalar inputs (I personally like using scalars when doing some exploratory work). My recommendation is to replace this loop with:

mask = sky_diffuse == 0
if isinstance(sky_diffuse, pd.Series):
    diffuse_components = pd.DataFrame(diffuse_components)
    diffuse_components.ix[mask] = 0
else:
    diffuse_components = {k: np.where(mask, 0, v) for k, v in diffuse_components.items()}

This has the unfortunate side effect of promoting a scalar to an array, but at least it's consistent with the rest of the api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.


return sky_diffuse, diffuse_components

else:
return sky_diffuse


def disc(ghi, zenith, datetime_or_doy, pressure=101325):
Expand Down
24 changes: 24 additions & 0 deletions pvlib/test/test_irradiance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,30 @@ def test_perez():
assert_series_equal(out, expected, check_less_precise=2)


def test_perez_components():
am = atmosphere.relativeairmass(ephem_data['apparent_zenith'])
dni = irrad_data['dni'].copy()
dni.iloc[2] = np.nan
out, components = irradiance.perez(40, 180, irrad_data['dhi'], dni,
dni_et, ephem_data['apparent_zenith'],
ephem_data['azimuth'], am, return_components=True)
expected = pd.Series(np.array(
[ 0. , 31.46046871, np.nan, 45.45539877]),
index=times)
expected_components = pd.DataFrame(
{
'circumsolar': np.array([ 0. , 0. , np.nan, 4.47966439]),
'isotropic': np.array([ 0. , 26.84138589, np.nan, 31.72696071]),
'horizon': np.array([ 0. , 4.62212181, np.nan, 9.25316454])
},
index=times
)
df_components = pd.DataFrame(components)
Copy link
Member

Choose a reason for hiding this comment

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

components should already be a dataframe, so this line is no longer necessary.

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 think it is not always a dataframe right now:

In [5]: irradiance.perez(15, 0, 100, 1000, 1300, 0, 0, 1, return_components=True)
Out[5]: 
(array(98.4801177930642),
 {'circumsolar': array(63.06009605934864),
  'horizon': array(1.2960861412441615),
  'isotropic': array(34.123935592471405)})

But I'll make sure it is!

Copy link
Member

Choose a reason for hiding this comment

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

Your test function uses a series as an input, so it gets a DataFrame as an output. I think the perez function itself currently has the correct behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I just got it, you don't want it as a dataframe in all cases!

Copy link
Member

Choose a reason for hiding this comment

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

Right. pandas in, pandas out. Otherwise, keep the types as similar as is practical.

sum_components = pd.Series(np.sum(list(components.values()), axis=0), index=times)
assert_series_equal(out, expected, check_less_precise=2)
assert_frame_equal(df_components, expected_components)
assert_series_equal(sum_components, expected, check_less_precise=2)

@needs_numpy_1_10
def test_perez_arrays():
am = atmosphere.relativeairmass(ephem_data['apparent_zenith'])
Expand Down