Skip to content

DOC: Discuss Angstrom Kasten TL, set default alpha #563

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

Merged
merged 10 commits into from
Sep 7, 2018

Conversation

mikofski
Copy link
Member

@mikofski mikofski commented Sep 6, 2018

pvlib python pull request guidelines

Thank you for your contribution to pvlib python! You may delete all of these instructions except for the list below.

You may submit a pull request with your code at any stage of completion.

The following items must be addressed before the code can be merged. Please don't hesitate to ask for help if you're unsure of how to accomplish any of the items below:

  • Closes add discussion and examples of Angstrom and Kasten formulas to docs #302
  • I am familiar with the contributing guidelines.
  • Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests (usually) must pass on the TravisCI and Appveyor testing services.
  • Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate docs/sphinx/source/whatsnew file for all changes.
  • Code quality and style is sufficient. Passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
  • Pull request is nearly complete and ready for detailed review.

Brief description of the problem and proposed solution (if not already fully described in the issue linked to above):

* closes pvlib#302
* add references to Kasten 96 and Molineaux
* discuss using Kasten and calculating broadband AOD usinge Angstrom turbidity model

Signed-off-by: Mark Mikofski <[email protected]>
@mikofski
Copy link
Member Author

mikofski commented Sep 6, 2018

is this okay?

@mikofski mikofski changed the title Discuss angstrom kasten tl DOC: Discuss angstrom kasten tl Sep 6, 2018

In [1]: MBARS = 100 # conversion factor from mbars to Pa

In [1]: URL = 'http://rredc.nrel.gov/solar/old_data/nsrdb/1991-2005/data/tmy3/722040TYA.CSV'
Copy link
Member

Choose a reason for hiding this comment

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

There is a TMY file in pvlib/data, I think it is safer to use it rather than read from the URL.

@mikofski
Copy link
Member Author

mikofski commented Sep 6, 2018

Great feedback, thanks. I agree with all comments.

FYI: There are no italics in the IPython example, that's just GitHub not rendering correctly b/c it doesn't understand the .. ipython:: directive, okay?

@mikofski
Copy link
Member Author

mikofski commented Sep 6, 2018

@wholmgren is there a built version of the docs that I can point reviewers to online?

@wholmgren
Copy link
Member

is there a built version of the docs that I can point reviewers to online?

You'd need to login to readthedocs.org and configure it to build this particular branch of your fork. It only takes a few minutes to set up the first time.

@wholmgren wholmgren added this to the 0.6.0 milestone Sep 6, 2018
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

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

thanks mark. minor edits suggested.


.. ipython::

In [1]: MBARS = 100 # conversion factor from mbars to Pa
Copy link
Member

Choose a reason for hiding this comment

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

I know you have your reasons for using caps here, but I think it's better for most readers to see lower case.

Solis model requires AOD at 700 nm. Models exist to convert AOD between
different wavelengths, as well as convert Linke turbidity to AOD and PW
[Ine08con]_, [Ine16]_.
Solis model requires AOD at 700 nm. The function,
Copy link
Member

Choose a reason for hiding this comment

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

delete The function and commas. I think the formatting makes it clear that it's a function. But you can keep it if you prefer this way.

Solis model requires AOD at 700 nm. The function,
:py:func:`~pvlib.atmosphere.angstrom_aod_at_lambda`, is useful for converting
AOD between different wavelengths using the Angstrom turbidity model and given
the Angstrom exponent, :math:`\alpha`, which can be calculated from AOD at two
Copy link
Member

Choose a reason for hiding this comment

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

please split this into 2 or more sentences.

AOD between different wavelengths using the Angstrom turbidity model and given
the Angstrom exponent, :math:`\alpha`, which can be calculated from AOD at two
wavelengths with the :py:func:`~pvlib.atmosphere.angstrom_alpha` function. The
function, :py:func:`~pvlib.atmosphere.bird_hulstrom80_aod_bb`, can be used to
Copy link
Member

Choose a reason for hiding this comment

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

same comment on "the function" as above

* use TMY in pvlib/data, lowercase MBARS, use non-specific variable names
* split output from readtmy3 instead of using indexing for clarity
@stickler-ci
Copy link

stickler-ci bot commented Sep 6, 2018

I couldn't find a .stickler.yml file in this repository. I can make one for you, or you can create one by following the documentation.

@mikofski
Copy link
Member Author

mikofski commented Sep 6, 2018

FYI: rtfd build of my fork is failing and I don't know why

@mikofski
Copy link
Member Author

mikofski commented Sep 6, 2018

Also there is currently no default for alpha in pvlib.atmosphere.angstrom_aod_at_lambda, but I think we should set it at 1.14 as recommended by NREL SPECTRL2 (see section 2)

When a single value of α is used to represent the rural aerosol model, the value should be α = 1.140.

It's also in the source for the origin FORTRAN code

	C   ALPHA= POWER ON ANGSTRON TURBIDITY EXPRESSION (1.14 FOR RURAL)

And in both spreadsheet implementation and C-headers, see here.

@wholmgren
Copy link
Member

@mikofski I'm not sure either. I've never seen that error. The common errors I see are intermittent and related to timeouts of one thing or another. I poked around the settings a little bit but nothing jumped out. Sorry.

@wholmgren
Copy link
Member

Fine with me if you want to make a default alpha. If so, please update whatsnew and the PR title accordingly.

@mikofski mikofski changed the title DOC: Discuss angstrom kasten tl DOC: Discuss Angstrom Kasten TL, set default alpha Sep 6, 2018
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

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

I rendered the documentation and it mostly looks good.

...: altitude=tmy_header['altitude'], pressure=tmy_data['Pressure']*mbars,
...: temperature=tmy_data['DryBulb'])

In [1]: am_rel = atmosphere.relativeairmass(solpos.apparent_zenith)
Copy link
Member

Choose a reason for hiding this comment

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

get_relative_airmass


In [1]: am_rel = atmosphere.relativeairmass(solpos.apparent_zenith)

In [1]: am_abs = atmosphere.absoluteairmass(am_rel, tmy_data['Pressure']*mbars)
Copy link
Member

Choose a reason for hiding this comment

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

get_absolute_airmass

In [1]: tmy_data, tmy_header = tmy.readtmy3(tmy_file) # read TMY data

In [1]: dt = pd.DatetimeIndex(start='1999-01-01 00:00:00', end='1999-12-31 23:59:59',
...: freq='H')
Copy link
Member

Choose a reason for hiding this comment

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

out of curiosity why do this instead of use the readtmy3 coerce_year kwarg?

@@ -118,6 +116,66 @@ The Ineichen and Perez clear sky model parameterizes irradiance in terms
of the Linke turbidity [Ine02]_. pvlib-python implements this model in
the :py:func:`pvlib.clearsky.ineichen` function.


The :py:func:`~pvlib.atmosphere.kasten96_lt` function can be used to calculate
Copy link
Member

Choose a reason for hiding this comment

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

The "Turbidity data" section below introduces the clearsky.lookup_linke_turbidity function. Your example uses this function, so I think your new text/example should be appended to that section. Am I missing something?


In [1]: alpha_exponent = atmosphere.angstrom_alpha(aod1240nm, 1240, aod550nm, 550)

In [1]: aod700nm = atmosphere.angstrom_aod_at_lambda(aod1240nm, 1240, alpha_exponent, 700)
Copy link
Member

Choose a reason for hiding this comment

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

suggest a line(s) that prints the alpha_exponent and aod700nm

@wholmgren wholmgren merged commit ec329e2 into pvlib:master Sep 7, 2018
@mikofski mikofski deleted the discuss_angstrom_kasten_tl branch September 8, 2018 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants