NREL bird clearsky #276

mikofski · 2016-12-03T10:27:51Z

Yet another clear sky model, based on the MS Excel workbook by NREL Daryl Myers and the model described in the paper "A Simplified Clear Sky model for Direct and Diffuse
Insolation on Horizontal Surfaces" by R.E. Bird and R.L Hulstrom, SERI Technical
Report SERI/TR-642-761, Feb 1981. Solar Energy Research Institute, Golden, CO.

* move clearsky.py into clearsky/core.py just for fun, let's try it out * fix path to data in clearsky to LinkeTurbidities.mat * add BIRD workbook to data for testing Signed-off-by: Mark Mikofski <[email protected]>

wholmgren

Thanks! I think the Bird model would be a useful addition, though I admit I don't have any experience with it.

I'll put the API-level comments here. The resolution of these might make some of my line-by-line comments irrelevant.

Restructuring clearsky.py into a package: maybe we should do this first in a separate PR? On a related note, I thought you were an proponent of api.py in modules are too long and have too many symbols #235 so I was a little surprised that you didn't use that here. It's not clear to me why bird would be in its own module while the other algorithms remain in the same module.
I'm not sure about the nrel.py module name. Do people frequently associate this algorithm with NREL?
The bird function does a lot: calculates solar position, airmass, atmospheric properties, ET irradiance, and finally clear sky. I think the function and its signature should be simplified by requiring users to do more work before they get here. This would also be consistent with the way the existing clear sky algorithms work. So, I think the signature should be more like

bird(zenith, airmass_relative, aod700=0.1, precipitable_water=1., ozone=0.3, pressure=101325., dni_extra=1364.)

This is also related to the ineichen refactoring that I did in #199. The documentation can make note of the methods that need to be used on the input data in order to exactly reproduce the traditional model.

Should we strip out the test data from the excel file and put it in a csv file? Advantages are that the csv file is smaller and we avoid the xlrd dependency. More importantly, are we allowed to redistribute the NREL file?

Why add the requirements file? setup.py already has the minimum pvlib requirements in it.

wholmgren · 2016-12-03T20:49:13Z

pvlib/clearsky/nrel.py

+def bird(doy, hr, lat, lon, tz, press_mB, o3_cm, h2o_cm, aod_500nm, aod_380nm,
+         b_a, alb, lyear=365.0, solar_constant=1367.0):
+    """
+    Bird Simple Clearsky Model


use numpydoc

wholmgren · 2016-12-03T20:49:52Z

pvlib/clearsky/nrel.py

+        np.sin(dec_rad) * np.sin(lat_rad)
+    )
+    zenith = np.rad2deg(ze_rad)
+    airmass = np.where(zenith < 89,


This is the same as relativeairmass(method='kasten1966')

removed this calc in bird since passing relative airmass as argument

use this in test_clearsky.test_bird()
✔️

wholmgren · 2016-12-03T20:52:50Z

pvlib/clearsky/nrel.py

+    return pd.DataFrame({'Eb': Eb, 'Ebh': Ebh, 'Gh': Gh, 'Dh': Dh}, index=dt)
+
+
+if __name__ == "__main__":


Hm, we don't have any script code in pvlib, but I would consider keeping this in a test module.

removed script - it was just for debugging
✔️

wholmgren · 2016-12-03T20:53:06Z

pvlib/clearsky/nrel.py

+    return solar_constant * (a0 + a1 + a2 + a3 + a4)
+
+
+def test_bird():


tests should go in a module named test_[__name__].py. We should probably make a tests subsubpackage within each subpackage e.g. pvlib/clearsky/tests/test_bird.py

wholmgren · 2016-12-03T20:54:26Z

pvlib/clearsky/nrel.py

+    return id_, id_nh, gh, gh - id_nh, testvalues
+
+
+def etr(doy, lyear=365.0, solar_constant=1367.0):


This the same as irradiance.extraradiation(method='spencer')

using dni_extra arg now, so this is removed, but I'll use it in the test
✔️

wholmgren · 2016-12-03T20:57:01Z

pvlib/clearsky/nrel.py

+    )
+    pstar = press_mB / patm
+    am_press = pstar*airmass
+    t_rayliegh = np.where(airmass > 0,


Are any of the np.where statements repetitive? In any case, calculate the airmass > 0 mask once and reuse it in subsequent tests.

not necessary anymore, since relative airmass is NaN if not valid
✔️

wholmgren · 2016-12-03T21:00:54Z

pvlib/clearsky/nrel.py

+    )
+    gh = np.where(airmass > 0, (id_nh + ias) / (1.0 - alb * rs), 0.0)
+    testvalues = (day_angle, declination, eqt, hour_angle, zenith, airmass)
+    return id_, id_nh, gh, gh - id_nh, testvalues


Better to do the subtraction on a separate line.

okay, changed to diffuse_horiz
✔️

wholmgren · 2016-12-03T21:38:45Z

pvlib/clearsky/nrel.py

+        Solar Radiation Model 
+From the publication "A Simplified Clear Sky model for Direct and Diffuse
+Insolation on Horizontal Surfaces" by R.E. Bird and R.L Hulstrom, SERI Technical
+Report SERI/TR-642-761, Feb 1991. Solar Energy Research Institute, Golden, CO.


wholmgren · 2016-12-03T21:41:37Z

pvlib/clearsky/nrel.py

+Insolation on Horizontal Surfaces" by R.E. Bird and R.L Hulstrom, SERI Technical
+Report SERI/TR-642-761, Feb 1991. Solar Energy Research Institute, Golden, CO.
+
+The model is based on comparisons with results from rigourous radiative transfer


text needs to be edited so that it's not specific to the excel file. Not sure if there is a licensing/attribution issue here.

wholmgren · 2016-12-03T21:43:16Z

pvlib/clearsky/nrel.py

+
+def bird(doy, hr, lat, lon, tz, press_mB, o3_cm, h2o_cm, aod_500nm, aod_380nm,
+         b_a, alb, lyear=365.0, solar_constant=1367.0):
+    """


Some (all?) of the edited doc string above should be moved here.

jforbess · 2016-12-05T19:18:29Z

Also not a fan of a module called nrel.py for this purpose, and I think that the bird model should be included in clearsky.py. Not sure about the new package concept for clearsky.py, but if it is, would prefer the module name to be bird or include bird.

…

On Sat, Dec 3, 2016 at 1:43 PM, Will Holmgren ***@***.***> wrote: ***@***.**** commented on this pull request. Thanks! I think the Bird model would be a useful addition, though I admit I don't have any experience with it. I'll put the API-level comments here. The resolution of these might make some of my line-by-line comments irrelevant. 1. Restructuring clearsky.py into a package: maybe we should do this first in a separate PR? On a related note, I thought you were an proponent of api.py in #235 <#235> so I was a little surprised that you didn't use that here. It's not clear to me why bird would be in its own module while the other algorithms remain in the same module. 2. I'm not sure about the nrel.py module name. Do people frequently associate this algorithm with NREL? 3. The bird function does a lot: calculates solar position, airmass, atmospheric properties, ET irradiance, and finally clear sky. I think the function and its signature should be simplified by requiring users to do more work before they get here. This would also be consistent with the way the existing clear sky algorithms work. So, I think the signature should be more like bird(zenith, airmass_relative, aod700=0.1, precipitable_water=1., ozone=0.3, pressure=101325., dni_extra=1364.) This is also related to the ineichen refactoring that I did in #199 <#199>. The documentation can make note of the methods that need to be used on the input data in order to exactly reproduce the traditional model. Should we strip out the test data from the excel file and put it in a csv file? Advantages are that the csv file is smaller and we avoid the xlrd dependency. More importantly, are we allowed to redistribute the NREL file? Why add the requirements file? setup.py already has the minimum pvlib requirements in it. ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > +http://rredc.nrel.gov/solar/models/clearsky/ +http://rredc.nrel.gov/solar/pubs/pdfs/tr-642-761.pdf +http://rredc.nrel.gov/solar/models/clearsky/error_reports.html +""" + +import numpy as np +import pandas as pd +import seaborn as sns +import xlrd +import os + + +def bird(doy, hr, lat, lon, tz, press_mB, o3_cm, h2o_cm, aod_500nm, aod_380nm, + b_a, alb, lyear=365.0, solar_constant=1367.0): + """ + Bird Simple Clearsky Model use numpydoc ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > + ) + declination = np.rad2deg(dec_rad) + # equation of time + eqt = 229.18 * ( + 0.0000075 + 0.001868 * np.cos(day_angle) - + 0.032077 * np.sin(day_angle) - 0.014615 * np.cos(2.0 * day_angle) - + 0.040849 * np.sin(2.0 * day_angle) + ) + hour_angle = 15.0 * (hr - 12.5) + lon - tz * 15.0 + eqt / 4.0 + lat_rad = np.deg2rad(lat) + ze_rad = np.arccos( + np.cos(dec_rad) * np.cos(lat_rad) * np.cos(np.deg2rad(hour_angle)) + + np.sin(dec_rad) * np.sin(lat_rad) + ) + zenith = np.rad2deg(ze_rad) + airmass = np.where(zenith < 89, This is the same as relativeairmass(method='kasten1966') ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > + for n, h in enumerate(headers)}, + index=dt[1:48]) + assert np.allclose(testdata['Dangle'], day_angle[1:48]) + assert np.allclose(testdata['DEC'], declination[1:48]) + assert np.allclose(testdata['EQT'], eqt[1:48]) + assert np.allclose(testdata['Hour Angle'], hour_angle[1:48]) + assert np.allclose(testdata['Zenith Ang'], zenith[1:48]) + assert np.allclose(testdata['Air Mass'], airmass[1:48]) + assert np.allclose(testdata['Direct Beam'], Eb[1:48]) + assert np.allclose(testdata['Direct Hz'], Ebh[1:48]) + assert np.allclose(testdata['Global Hz'], Gh[1:48]) + assert np.allclose(testdata['Dif Hz'], Dh[1:48]) + return pd.DataFrame({'Eb': Eb, 'Ebh': Ebh, 'Gh': Gh, 'Dh': Dh}, index=dt) + + +if __name__ == "__main__": Hm, we don't have any script code in pvlib, but I would consider keeping this in a test module. ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > + gh = np.where(airmass > 0, (id_nh + ias) / (1.0 - alb * rs), 0.0) + testvalues = (day_angle, declination, eqt, hour_angle, zenith, airmass) + return id_, id_nh, gh, gh - id_nh, testvalues + + +def etr(doy, lyear=365.0, solar_constant=1367.0): + a0 = 1.00011 + doy0 = doy - 1.0 + a1 = 0.034221 * np.cos(2.0 * np.pi * doy0 / lyear) + a2 = 0.00128 * np.sin(2.0 * np.pi * doy0 / lyear) + a3 = 0.000719 * np.cos(2.0 * (2.0 * np.pi * doy0 / lyear)) + a4 = 0.000077 * np.sin(2.0 * (2.0 * np.pi * doy0 / lyear)) + return solar_constant * (a0 + a1 + a2 + a3 + a4) + + +def test_bird(): tests should go in a module named test_[__name__].py. We should probably make a tests subsubpackage within each subpackage e.g. pvlib/clearsky/tests/test_bird.py ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > + 0.9662 * etr_ * t_aerosol * t_water * t_gases * t_ozone * t_rayliegh, + 0.0 + ) + id_nh = np.where(zenith < 90, id_ * np.cos(ze_rad), 0.0) + ias = np.where(airmass > 0, + etr_ * np.cos(ze_rad) * 0.79 * t_ozone * t_gases * t_water * taa * + (0.5 * (1.0 - t_rayliegh) + b_a * (1.0 - (t_aerosol / taa))) / ( + 1.0 - airmass + airmass ** 1.02 + ), 0.0 + ) + gh = np.where(airmass > 0, (id_nh + ias) / (1.0 - alb * rs), 0.0) + testvalues = (day_angle, declination, eqt, hour_angle, zenith, airmass) + return id_, id_nh, gh, gh - id_nh, testvalues + + +def etr(doy, lyear=365.0, solar_constant=1367.0): This the same as irradiance.extraradiation(method='spencer') ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > + 0.032077 * np.sin(day_angle) - 0.014615 * np.cos(2.0 * day_angle) - + 0.040849 * np.sin(2.0 * day_angle) + ) + hour_angle = 15.0 * (hr - 12.5) + lon - tz * 15.0 + eqt / 4.0 + lat_rad = np.deg2rad(lat) + ze_rad = np.arccos( + np.cos(dec_rad) * np.cos(lat_rad) * np.cos(np.deg2rad(hour_angle)) + + np.sin(dec_rad) * np.sin(lat_rad) + ) + zenith = np.rad2deg(ze_rad) + airmass = np.where(zenith < 89, + 1.0 / (np.cos(ze_rad) + 0.15 / (93.885 - zenith) ** 1.25), 0.0 + ) + pstar = press_mB / patm + am_press = pstar*airmass + t_rayliegh = np.where(airmass > 0, Are any of the np.where statements repetitive? In any case, calculate the airmass > 0 mask once and reuse it in subsequent tests. ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > + ) + etr_ = etr(doy, lyear, solar_constant) + id_ = np.where(airmass > 0, + 0.9662 * etr_ * t_aerosol * t_water * t_gases * t_ozone * t_rayliegh, + 0.0 + ) + id_nh = np.where(zenith < 90, id_ * np.cos(ze_rad), 0.0) + ias = np.where(airmass > 0, + etr_ * np.cos(ze_rad) * 0.79 * t_ozone * t_gases * t_water * taa * + (0.5 * (1.0 - t_rayliegh) + b_a * (1.0 - (t_aerosol / taa))) / ( + 1.0 - airmass + airmass ** 1.02 + ), 0.0 + ) + gh = np.where(airmass > 0, (id_nh + ias) / (1.0 - alb * rs), 0.0) + testvalues = (day_angle, declination, eqt, hour_angle, zenith, airmass) + return id_, id_nh, gh, gh - id_nh, testvalues Better to do the subtraction on a separate line. ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > @@ -0,0 +1,187 @@ +""" +Richard E. Bird + Clear Sky Broadband + Solar Radiation Model +From the publication "A Simplified Clear Sky model for Direct and Diffuse +Insolation on Horizontal Surfaces" by R.E. Bird and R.L Hulstrom, SERI Technical +Report SERI/TR-642-761, Feb 1991. Solar Energy Research Institute, Golden, CO. 1981 ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > @@ -0,0 +1,187 @@ +""" +Richard E. Bird + Clear Sky Broadband + Solar Radiation Model +From the publication "A Simplified Clear Sky model for Direct and Diffuse +Insolation on Horizontal Surfaces" by R.E. Bird and R.L Hulstrom, SERI Technical +Report SERI/TR-642-761, Feb 1991. Solar Energy Research Institute, Golden, CO. + +The model is based on comparisons with results from rigourous radiative transfer text needs to be edited so that it's not specific to the excel file. Not sure if there is a licensing/attribution issue here. ------------------------------ In pvlib/clearsky/nrel.py <#276 (review)> : > + +http://rredc.nrel.gov/solar/models/clearsky/ +http://rredc.nrel.gov/solar/pubs/pdfs/tr-642-761.pdf +http://rredc.nrel.gov/solar/models/clearsky/error_reports.html +""" + +import numpy as np +import pandas as pd +import seaborn as sns +import xlrd +import os + + +def bird(doy, hr, lat, lon, tz, press_mB, o3_cm, h2o_cm, aod_500nm, aod_380nm, + b_a, alb, lyear=365.0, solar_constant=1367.0): + """ Some (all?) of the edited doc string above should be moved here. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#276 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH66AV_G_c7XNkhsM_6sTWTQPYQwK6waks5rEeKRgaJpZM4LDQh7> .

* compare eot from spencer and pvcdrom to spa-python * add note about coefficients in spencer 1971 and more references

* clean up docstrings for bird model, fix pub date to 1981, add references section, add attribution to daryl myers * add spaces between colon and parameter to ineichen and simplified_solis models to correspond to numpydoc style * revert changes in 457615d, change pvlib-path back to dirname of clearsky.py file, remove added clearsky_path * start to change bird function signature to conform to existing clear sky algos, and strip out some of the complicated algos like the relative airmass, declination and solpos * change args aod_380nm to aod380 and aod_500nm to aod500. * clean up eot docstrings, add references, use external linking

* added both cooper-69 and spencer-71 * replacing calls in bird to calls to new formulas for declination, eot, dayangle

* remove day_angle, relative airmass, eot, zenith, declination, extraterrestrial dni calcs, and pass values in as needed * change o3_cm to ozone, h2o_cm to precipitable_water, b_a to asymmetry and alb to albedo * replace pstar, patm and am_press with absolute airmass method from atmosphere * testvalues no longer needed * use irradiance.extraradiation() and remove etr() * add analytical solar zenith angle and hour angle methods to solar position module

* also add returns * remove defaults for aod, since should be supplied TODO: change input to broadband when linke turbidity PR is complete * remove times, not needed anymore * remove commented relative airmass(method="kasten1966") since passed as arg * remove extraradiation(method="spencer") since passed as arg * change etr_ to etr since no more etr TODO: change all variables to easier names * add seinfeld and pandis to declination references * finish hour angle calc

* save ze_cos once, instead of recalculating * simplify/shorten some lines now that np.where(airmass > 0,<calc>,0.0) is gone

* add to what's new for v0.4.4 * add new solar position formulas and bird clear sky to api * move links to references, and space references to make numpydoc happy * add see also for eot, declination and solar_zenith_analytical

mikofski · 2017-01-14T08:53:38Z

I think this is done. Some notes:

I reversed the API changes. I'll leave that to you. Personally I like to use the dunder classes for breaking up modules into packages, but the api.py idea seems interesting too.
I refactored the bird model to remove any redundant solarposition and atmosphere stuff, like you said. For testing I added some basic solar position stuff like analytical zenith, equation of time and declination
I also refactored the signature to match other clearsky models
if you really want to remove the xls bird file in lieu of a csv, that's fine, but it is a primary source, and I don't think there are any license issues, it is publicly available without registering, but guess we should check on that. I would think like 90% of users will already have xlrd, PITA for build testing huh
the requirements file is really really useful!
1. if you weren't using conda, travis would just look at your requirements and build your venv from that
2. it makes it really easy to make a venv, which is what I've done for pvlib and why I made the file. Most projects recommend it, EG see this
either python 3.4 or 3.5 randomly fails for no reason, something in test_forecast using netCDF4, but it doesn't always happen, and sometimes it changes from one version to another. What's up?

wholmgren · 2017-01-20T03:37:39Z

Sorry again for the delay @mikofski. Does anyone have time to look at this more carefully?

Without looking closely (and thus running a high chance of saying something stupid)...

I reversed the API changes.

API looks good, thanks. I like the idea of reorganizing with api.py or something else, but let's do it in a separate PR.

I refactored the bird model to remove any redundant solarposition and atmosphere stuff, like you said. For testing I added some basic solar position stuff like analytical zenith, equation of time and declination

I was initially unhappy to see all of this less precise solar position functionality in a clear sky model PR. But I guess I'm ok with these functions if we document that users should prefer other functions for high precision calculations. Maybe put them in a different subsection in the API documentation?

if you really want to remove the xls bird file in lieu of a csv, that's fine, but it is a primary source, and I don't think there are any license issues, it is publicly available without registering, but guess we should check on that. I would think like 90% of users will already have xlrd, PITA for build testing huh

I like plain text for smaller data because people can view it on github, in diffs, and their editors. But this is fine.

the requirements file is really really useful!

The base requirements are numpy 1.9, pandas 0.14, pytz, and six. Maybe you want to make two requirements files -- requirements and requirements-all? Why the newer numpy scipy version limits?

either python 3.4 or 3.5 randomly fails for no reason, something in test_forecast using netCDF4, but it doesn't always happen, and sometimes it changes from one version to another. What's up?

see #293.

Are the keyword parameters sufficiently tested? Does it work with scalars?

- use csv file instead of xls file - NOTE: I made this file by changing all cells' format to General, fitting column size to content width to show all significant figures, then saving as csv, new file is less than 370kb vs 1.2mb - use pandas read_csv instead of xlrd, way easier :) - remove requirements.txt, can't be bothered

mikofski · 2017-01-21T01:40:11Z

still working on it. Any comments from anyone?

Maybe put them in a different subsection in the API documentation

good idea, I'll put them in a section after spa with text prefixing that says these are low accuracy solar position methods

I like plain text for smaller data because people can view it on github, in diffs, and their editors

remove xls, and reversed xlrd dependencies everywhere and use csv instead

The base requirements are numpy 1.9, pandas 0.14, pytz, and six. Maybe you want to make two requirements files -- requirements and requirements-all? Why the newer numpy scipy version limits?

I removed the requirements.txt, too much trouble if no one cares but me, the version dependencies were accidental, but there were other dependencies to build sphinx for example, and ephem, etc, it's just not worth it

Are the keyword parameters sufficiently tested? Does it work with scalars?

Nope, still need to check on this

* add Daryl R. Myers, "Solar Radiation" CRC Press (2013) reference * fix some sphinx-build errors in solar-position, explicit emphasis start with no end for "*Note" (missing closing "*" for emphasis in `nrel_earthsun_distance` and `spa_python` * use numpydoc linked references for spencer71 equation of time

* add BIRD_08_16_2012_patm.csv for 2nd bird test data * fix docs by moving text before parameters so not indented

mikofski · 2017-01-23T23:08:03Z

@wholmgren and others:

This is ready for your review now:

moved low precision solar position correlations to separate section of documentation on API.rst
added keyword and scalar tests - passing

wholmgren · 2017-01-24T00:32:56Z

Thanks, Mark. Might be next week before I can look carefully. Other users should feel free to chime in.

wholmgren

Sorry for long delay in getting to this @mikofski. I think we're close.

It would be great if you could add a new section to the clearsky.rst file and update the other sections appropriately, but I worry that might be asking too much for this PR. We could leave it as a stub in the documentation and create a separate issue to remind us to get to it eventually.

wholmgren · 2017-02-03T21:11:23Z

docs/sphinx/source/api.rst

+   solarposition.solar_zenith_analytical
+   solarposition.declination_spencer71
+   solarposition.declination_cooper69
+   solarposition.equation_of_time_Spencer71


lower case s here and in function definition.

wholmgren · 2017-02-03T21:15:23Z

pvlib/clearsky.py

+    Parameters
+    ----------
+    zenith : numeric
+        Solar zenith angle in degrees.


solar_zenith or apparent_zenith? http://pvlib-python.readthedocs.io/en/latest/variables_style_rules.html

solar_zenith not apparent - in case I forget

wholmgren · 2017-02-03T21:20:25Z

pvlib/clearsky.py

+    NREL Bird Simple Clear Sky Broadband Solar Radiation Model
+
+    Based on original implementation by Daryl R. Myers at NREL.
+


You could add some notes about how zenith and airmass should be calculated if users want to exactly reproduce the entire bird model.

Or this could go into a new section in the clearsky.rst documentation.

wholmgren · 2017-02-03T21:24:19Z

pvlib/clearsky.py

+
+    Returns
+    -------
+    direct_beam : numeric


It's more conventional in pvlib to return a DataFrame or OrderedDict, rather than a tuple. Series in, DataFrame out, anything else in, OrderedDict out. The other functions in this module demonstrate this pattern. columns/keys should follow http://pvlib-python.readthedocs.io/en/latest/variables_style_rules.html

mikofski · 2017-02-28T19:40:49Z

@wholmgren sorry for being so slow. I am still working on this.

mikofski · 2017-05-17T07:23:00Z

@wholmgren and @cwhanse I think this is finally ready to be merged. It passes all tests. I've addressed the return types by converting them to OrderdDict or DataFrame and ordered the output similar to other clear sky models. I did not address the concerns about adding documentation to clearsky.rst about the inputs to duplicate the Daryl Myers Excel spreadsheet, or add any new significant documentation other that to the API. Perhaps I can address additional documentation as needed in a future realease? Thanks!

mikofski · 2017-05-18T21:44:51Z

@wholmgren I made minor documentation changes to address the testing parameters and solar or apparent zenith argument. Please let me know if there are any other issues to address before you can merge this. I would like to use the Bird model for comparisons of AOD/Pwat data with Linke turbidity in with Ineichen's original and newer simplified-Solis clearsky model for our upcoming PVSC paper. 😉

wholmgren · 2017-05-18T21:57:43Z

That all sounds good, but I probably will not be able to look at this closely until next wednesday.

…

On Thu, May 18, 2017 at 2:44 PM Mark Mikofski ***@***.***> wrote: @wholmgren <https://github.com/wholmgren> I made minor documentation changes to address the testing parameters and solar or apparent zenith argument. Please let me know if there are any other issues to address before you can merge this. I would like to use the Bird model for comparisons of AOD/Pwat data with Linke turbidity in with Ineichen's original and newer simplified-Solis clearsky model for our upcoming PVSC paper. 😉 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#276 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AELiR2G6PWT34mBjPNJkVjl1eRUMAWF1ks5r7LvTgaJpZM4LDQh7> .

wholmgren

How big (MB) are these test files?

wholmgren · 2017-05-24T22:55:45Z

docs/sphinx/source/whatsnew/v0.4.4.txt

@@ -6,6 +6,7 @@ v0.4.4 (February 18, 2017)
 Enhancements
 ~~~~~~~~~~~~

+* Added NREL Bird clear sky model. (:issue:`276`)


we're onto 0.4.5

wholmgren · 2017-05-24T22:58:40Z

pvlib/clearsky.py

+    airmass = airmass_relative
+    # Bird clear sky model
+    am_press = atmosphere.absoluteairmass(airmass, pressure)
+    t_rayliegh = (


rayleigh not rayliegh

wholmgren · 2017-05-24T23:04:56Z

pvlib/test/test_solarposition.py

@@ -367,3 +367,15 @@ def test_nrel_earthsun_distance():
    expected = pd.Series(np.array([0.983289204601]),
                         index=pd.DatetimeIndex([times, ]))
    assert_series_equal(expected, result)
+


shouldn't we add tests for all of the other functions you've added to solarposition.py? I think they should be marked private if you don't want to add the tests.

Added 2 more tests for low accuracy declination and solar-zenith formulas.

mikofski · 2017-05-25T03:35:03Z

@wholmgren the test files are each 360-kb, so total combined less than 1-mb, okay?

* move whats new note from 0.4.4 to 0.4.5 * test declination and solar-zenith * correct spelling of Rayleigh

wholmgren · 2017-05-25T14:17:11Z

ok on the test file size.

I think this is ready to go. Agreed?

mikofski · 2017-05-25T14:55:26Z

Unless anyone else has any comments, yes!

mikofski added 2 commits December 3, 2016 02:17

add requirements.txt to install into virtualenv and ignore venv/

9d192ba

add Bird clearsky model

457615d

* move clearsky.py into clearsky/core.py just for fun, let's try it out * fix path to data in clearsky to LinkeTurbidities.mat * add BIRD workbook to data for testing Signed-off-by: Mark Mikofski <[email protected]>

wholmgren reviewed Dec 3, 2016

View reviewed changes

mikofski added 17 commits December 15, 2016 15:20

move test_bird to test_clearsky and move bird to clearsky core module

048a837

revert api changes

1324267

add spencer and pvcdron eot to solarposition, fix test and 2pi

f985b15

add equation of time test

3ace2a8

* compare eot from spencer and pvcdrom to spa-python * add note about coefficients in spencer 1971 and more references

add declination formulas to solarposition.py

3b86fd8

* added both cooper-69 and spencer-71 * replacing calls in bird to calls to new formulas for declination, eot, dayangle

set night values to zero

8877fb0

* save ze_cos once, instead of recalculating * simplify/shorten some lines now that np.where(airmass > 0,<calc>,0.0) is gone

calculate diffuse_horiz return value prior to return line

de0eb33

update test_bird() with new function signature

da8e292

Merge branch 'master' into nrel_bird_clearsky

b8d93cf

Merge branch 'master' into nrel_bird_clearsky

0e69eca

update docs

dd97ecc

* add to what's new for v0.4.4 * add new solar position formulas and bird clear sky to api * move links to references, and space references to make numpydoc happy * add see also for eot, declination and solar_zenith_analytical

add missing requirements

96af6a7

add it to min build too

926c28b

wholmgren mentioned this pull request Jan 14, 2017

Simple analytical Azimuth method #291

Closed

mikofski mentioned this pull request Jan 14, 2017

Combine declination formulas into one like airmass #295

Open

mikofski added 3 commits January 23, 2017 13:16

test keywords in bird clear sky model

977bc8f

* add BIRD_08_16_2012_patm.csv for 2nd bird test data * fix docs by moving text before parameters so not indented

test scalars in bird model

09d0b7e

add test data file

cb562ef

wholmgren reviewed Feb 3, 2017

View reviewed changes

mikofski added 3 commits February 4, 2017 01:33

Merge branch 'master' into nrel_bird_clearsky

1976721

lower case Spencer71 in docs and solarposition

bc5eb18

Merge branch 'master' into nrel_bird_clearsky

a4b9edd

wholmgren added this to the 0.4.5 milestone Feb 15, 2017

mikofski added 4 commits May 16, 2017 17:11

Merge branch 'master' into nrel_bird_clearsky

76db3c7

fix lower case 's' in spencer71

513cd95

output OrderedDict or DataFrame

dc528be

fix link

ca8adf8

mikofski added 2 commits May 18, 2017 13:17

update docstring returns to OrderedDict/DataFrame

1e14e5f

provide note on solar vs apparent zenith and testing

b4e5cce

quotes for 'direct_horizontal' in docstring returns

2a0458a

wholmgren reviewed May 24, 2017

View reviewed changes

add tests for solarposition low-accuracy formulas

eafb9cd

* move whats new note from 0.4.4 to 0.4.5 * test declination and solar-zenith * correct spelling of Rayleigh

wholmgren merged commit b4db6e2 into pvlib:master May 26, 2017

mikofski deleted the nrel_bird_clearsky branch May 26, 2017 19:09

mikofski mentioned this pull request Jun 30, 2017

use generic broadband AOD in Bird model instead of only Bird-Hulstrom approximation #341

Open

		return pd.DataFrame({'Eb': Eb, 'Ebh': Ebh, 'Gh': Gh, 'Dh': Dh}, index=dt)


		if __name__ == "__main__":

		return solar_constant * (a0 + a1 + a2 + a3 + a4)


		def test_bird():

		return id_, id_nh, gh, gh - id_nh, testvalues


		def etr(doy, lyear=365.0, solar_constant=1367.0):

		NREL Bird Simple Clear Sky Broadband Solar Radiation Model

		Based on original implementation by Daryl R. Myers at NREL.

NREL bird clearsky #276

NREL bird clearsky #276

Conversation

mikofski commented Dec 3, 2016 • edited Loading

wholmgren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jforbess commented Dec 5, 2016 via email

mikofski commented Jan 14, 2017 • edited Loading

wholmgren commented Jan 20, 2017

mikofski commented Jan 21, 2017

mikofski commented Jan 23, 2017

wholmgren commented Jan 24, 2017

wholmgren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikofski commented Feb 28, 2017

mikofski commented May 17, 2017

mikofski commented May 18, 2017

wholmgren commented May 18, 2017 via email

wholmgren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikofski commented May 25, 2017

wholmgren commented May 25, 2017

mikofski commented May 25, 2017

mikofski commented Dec 3, 2016 •

edited

Loading

mikofski commented Jan 14, 2017 •

edited

Loading