Skip to content
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

migrate to pvlib>=0.7.0 #170

Merged
merged 7 commits into from Apr 29, 2020
Merged

migrate to pvlib>=0.7.0 #170

merged 7 commits into from Apr 29, 2020

Conversation

kandersolar
Copy link
Member

See #143. This PR updates how we use the sapm_effective_irradiance and sapm_cell functions to match the changes introduced in pvlib 0.7.0. Note that since the notebook calculates cell temperature, it also needed updates (no change in results).

Side note -- I would be interested to learn why I get warnings when running the notebook with the notebook environment but apparently other people do not. Need to get rid of those warnings before merging this.

@kandersolar kandersolar added this to the Version 2 milestone Apr 22, 2020
@kandersolar
Copy link
Member Author

@cwhanse - I think I've done these updates correctly, but if you are able to take a look to verify, that would be great.

.sapm_celltemp(irrad=total_irradiance['poa_global'],
wind=met_data['Wind Speed'],
temp=met_data['Temperature'])
.sapm_celltemp(poa_global=total_irradiance['poa_global'],
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes that pvlib_pvsystem.temperature_model_parameters are set, or can be inferred from pvlib_pvsystem.racking_model and pvlib_pvsystem.module_type. I see that racking_model is set, but I didn't see where module_type is set (values can be 'glass_polymer' and 'glass_glass', default 'glass_polymer'). The code should work as is, but to be explicit about the temperature model, I would suggest setting pvlib_pvsystem.module_type or setting pvlib_pvsystem.temperature_model_parameters if that's not already being done. No promises if the current default behavior will continue after pvlib v0.8.0.

That's the only thing I see.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I've updated the docstrings and tests to be explicit about the temperature model. I appreciate the review.

Copy link

@wholmgren wholmgren Apr 23, 2020

Choose a reason for hiding this comment

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

Note these are positional args, not keyword args, so I don't promise to deprecate the names if we decide to change them. In particular, I think temp_air is going to change at some point in the near future. So, you might want to call them using just the positions rather than the names.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @wholmgren. setup.py does specify pvlib<0.8.0, but might as well try to be forward-compatible if possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If things change in future versions of pvlib, is this a case where we can use conditionals to continue to support 0.7?

Copy link
Collaborator

Choose a reason for hiding this comment

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

On a related note: If in the future we change to requiring pvlib>0.8, but that doesn't change the RdTools calculations or API, it seems we don't need a major version. (See Semantic Versioning FAQ)

@mdeceglie
Copy link
Collaborator

mdeceglie commented Apr 23, 2020

@kanderso-nrel When I remember to, I delete the warnings from the raw notebook JSON before finalizing an update to the notebook. It gives the example a cleaner appearance, but perhaps it is confusing and we should leave them?

rdtools/normalization.py Outdated Show resolved Hide resolved
@kandersolar
Copy link
Member Author

I delete the warnings from the raw notebook JSON before finalizing an update to the notebook. It gives the example a cleaner appearance, but perhaps it is confusing and we should leave them?

To the extent that the notebook acts as a reproducibility test for the user, I guess warnings are a part of the expected output, so I think I'd be in favor of leaving them in the notebook. But maybe it would be better to just fix whatever is raising the warnings? I get two warnings with the current notebook:

  • pandas, register_matplotlib_converters()
  • pandas, Converting timezone-aware DatetimeArray to timezone-naive ndarray with 'datetime64[ns]' dtype. from the df.reindex in the clearsky temp function

Using the most recent pandas version gets rid of both warnings. And using an updated statsmodels gets rid of the third warning that the new pandas raises. 😉 Thoughts on bumping those requirements in a new PR?

@mdeceglie
Copy link
Collaborator

I'm in favor of bumping requirements to get rid of warnings

Copy link
Collaborator

@cdeline cdeline left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Collaborator

@mdeceglie mdeceglie left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@kandersolar kandersolar merged commit 09cea8a into development Apr 29, 2020
@kandersolar kandersolar deleted the pvlib_compat branch April 29, 2020 21:21
cdeline added a commit that referenced this pull request Jul 31, 2020
* configure travis to only deploy on one build

* prevent NaNs from summing to 0

* Update pandas requirement

min_count was added as an argument for dataframe.resample().sum() in 0.22.0

* add energy_from_power() and associated tests

* Update example notebook

Changes in step 0:
-use interpolate to resample the dataframe
-use new energy_from_power() function

* fix integer division bug for phython 2

* pep8 compliance

* Fix docstring spelling errors

* change name of series returned by energy_from_power()

* Update tests for expected series name

* Allow energy_from_power() with single argument

* change second argument of energy_from_power() to be a Timedelta (This is more appropriate and versatile than a float with implied units)

* Add series interpolation

* Add dataframe input capability to interpolate_to_index()

* Change function name and add frequency specification capability to interpolate()

* Speed up interpolate_series() (converting to integer index under the hood is faster than pandas timestamps)

* Run example with rdtools.interpolate() in step 0

* New version of energy_from_power and incorporation into normalization

* require pandas >= 0.23

* Remove warnings from interpolation when max_timedelta is inferred

* work around for pandas issue

* Add non-workaround case for tz naive series

* Change the behavior of interpolate to return NaNs where max_delta is exceeded

Old behavior was to have those times be missing from the output. The new behavior ensures that interpolate produces regular time series when a frequency is passed as target.

* address bug related to python 2 sting casting

* Add recomendation against using interpolation for downsampling

* Update notebook_requirements to address security issue in notebook version

* Update copyright line of LICENSE

* Update to use pvlib>0.6.0

* Add code from pv_soiling to soiling.py

Includes the addition of the wrapper function soiling_srr. The manual_cleanings keyword was also removed, it was not implimented in pv_soiling.

* Add confidence metric keywords and calc_info to soiling_srr()

* Avoid manipulating precip input name and change pandas syntax

* pass args to calc_monte() in soiling_srr()

* rename infer_clean to half_norm_clean

* Retain timezone in pm_frame creation

* Renormalize to median of first year (per yoy degradation).  Renorm factor is also included with calc_info

* Keep all the monte carlo soiling profiles

* import soiling_srr in __init__.py

* Add more to the soiling calc_info output

* enable modification of validity criteria in calc_result_frame

* Add validity parameters to soiling_srr()

* Clarify units of exceedance_prob in degradation_year_on_year

* add type to confidence_level in degradation_year_on_year

* clean up recenter type specification

* Update example notebook with soiling functionality

* Add axis labels to soiling plots

* Add random_seed to soiling_srr for repeatable results

* use true division in soiling.py

* require numpy greater than 1.12

Soiling code requires numpy distributions with scale 0, see numpy PR #5822 for more information

* Update notebook with note about get_clearsky() behavior change

Warnings were also removed from output, and the kernel was changed to `Python [default]`

* Replace hashes in soiling_test with more interpretable tests

* enforce column order in test_soiling_srr() test

* refactored soiling method conditionals

* Add soiling rate histogram to example

* Refactor soiling to single class

* change irradiance-weighted to insolation-weighted in soiling output

* add soiling histogram for readme

* Update readme with soiling information

* Rename the example notebook

* Add docstring to srr_analysis.__init__()

* Docstring standardization and pep8 formatting (#125)

* docstring formatting cleanup for sphinx

* add sphinx configuration files and index.rst

* document missing parameters for get_clearsky_tamb

* Add docstrings for undocumented functions.

* integrate ipython notebook examples with sphinx docs

* Update requirements.txt for Sphinx: m2r, nbsphinx, nbsphinx-link

* need to rev h5py, numpy and scipy to avoid setup install errors.

* add ipython to requirements.txt to allow inline ipython notebook examples

* rollback scipy to 1.2.2 (py27 compliant)

* custom .yml and requirements file for rtd

* point to index.html in conf.py

* test out path fixes for README.md to RST conversion

* Add Changelog to sphinx docs (#134)

* add sphinx.ext.extlinks to conf.py

* Drop py2.7 and add 3.7 and 3.8 to testing (#135)

* Drop 2.7 and add 3.7 and 3.8 to testing, update docs.

* creating DatetimeIndex directly is deprecated, switch to pd.date_range

* require pandas < 1.0.0

* bump requirements.txt numpy to 1.17.3 for testing on py3.8

* more requirements.txt updates for py3.8 wheel availability

* Update v2.0.0.rst

* convert README.md to ReSt in index.rst, move images folder into sphinx source

* Fix formatting, fix conf.py images folder

* Fix crosslinks and image size

* mention soiling

* another crosslink

* Improve module-level docstrings (#137)

* Improve module docstrings for sphinx docs module summaries

* Update v2.0.0.rst

* pare down readme.md

* incorporate new rdtools draft mission statement

* Add developer notes section to sphinx docs (#136)

* Add developer notes page and update setup.py with doc reqs

* improve formatting in developer_notes.rst

* missed a couple formatting issues

* change RTD to use setuptools extras instead of sphinx_requirements.txt

* add ipython to doc requirements

* change backslashes to forward slashes because they work on windows too.  also remove unnecessary note about ipython

* cleanup

* Update v2.0.0.rst

* add [test] requirements to setup.py

* update developer notes with more detailed installation instructions

* add code requirements section

* Add coverage instructions and configuration

* link typo

* simplify pytest instructions

* clarify installing rdtools from local git repo

* Bump bleach from 2.1.3 to 3.1.1 in /docs (#149)

Bumps [bleach](https://github.com/mozilla/bleach) from 2.1.3 to 3.1.1.
- [Release notes](https://github.com/mozilla/bleach/releases)
- [Changelog](https://github.com/mozilla/bleach/blob/master/CHANGES)
- [Commits](mozilla/bleach@v2.1.3...v3.1.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* rerun `versioneer install` to fix v-prefix issue (#153)

* Cherry-pick the plotting module out of the system analysis branch (#138)

* cherry-pick the plotting module commit

* add module-level docstring

* make plotting module docstrings numpydoc-compliant

* update deg/soiling example to use plotting functions

* add plotting module to api.rst

* fix crosslinks

* add matplotlib to setup.py

* update requirements.txt matplotlib for 3.8 testing

* update changelog

* add bins parameter to plotting.degradation_summary_plot from #132

* add basic plotting tests

* exercise all kwargs in plotting tests

* simplify pytest fixture; test kwargs in separate functions

* missed one

* fancy error messages

* Clean up errant spaces

* remove normalize_with_pvwatts import

Co-authored-by: Michael Deceglie <michael.deceglie@nrel.gov>
Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com>

* fix typo

* make introductions more approachable to non-specialists

* docs cleanup

* improve contributing notes

* drop m2r requirement since index.rst isn't generated from readme.md anymore

* remove m2r from conf.py

* requested changes

* hyphen

* Remove low power cutoff in filtering.clipping_filter (#144)

* remove low power cutoff in filtering.clipping_filter

* Update v2.0.0.rst

* shame on me for not running tests locally

* Update degradation_and_soiling_example.ipynb

* Update degradation_and_soiling_example.ipynb

* change normalized filter to > 0.01 per internal discussion

* Workaround for reindexing bug in Pandas 1.0.0 (#142)

* tentative workaround for pandas 1.0.0 reindexing bug

* Bump requirements to Pandas <= 1.0.0 for Travis upgrade-strategy=eager pytests

* update pandas req to exclude 1.0.0, 1.0.1

* Revert "tentative workaround for pandas 1.0.0 reindexing bug"

This reverts commit 9b69a9d.

* setup.py version lists are combined with AND, so <1.0.0, >=1.0.2 wasn't valid

Co-authored-by: cdeline <chris.deline@nrel.gov>

* Remove conflicting reqs (#164)

* Remove conflicting reqs

Some packages were specified in both notebook_requirements.txt and requirements.txt. In some cases versions conflicted.

* Update requirements.txt with indirect requirements

* Run notebook

* Add a note to first install requirements.txt

* Notebook updates

-Add directions to install both requirement files
-Incorporate bugfix from #166

* Changelog update

* Apply suggestions to changelog

Co-Authored-By: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

* Update normalization.irradiance_rescale and create irradiance_rescale_test.py (#152)

* Create irradiance_rescale_test.py

* pep8 spaces

* NameError -> UnboundLocalError

* expose convergence_threshold and default method to 'iterative'

* bugfix

* allow max_iterations=0

* bugfix

* changelog

Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com>

* Empty soiling intervals bugfix (#169)

* Closes #129 bug for invalid soiling intervals

* changelog entry

* Add a test to catch nan soiling interval bug

* clean up comment indentation

* SRR fixes (#154)

* fix precipitation frequency bug

* doc typo

* Normalized filter cherrypick (#139)

* Add normalized_filter() function to replace the mannual filter in example

* update docstring

* add entry to api.rst

* update changelog

* update lower bound to 0.01

* changelog for notebook changes

* add normalized_filter test to exercise default lower bound

Co-authored-by: Michael Deceglie <michael.deceglie@nrel.gov>

* update soiling docstrings to mention soiling stations and aggregation (#165)

* capitalize the SRR class name (#168)

* rename srr_analysis to SRRAnalysis

* formatting fix

* changelog

* Miscellaneous cleanups (#162)

* remove __future__ imports

* normalize_with_sapm duplicate lines

* setup.py status from beta to production/stable

* Delete build.bat

* copyright to 2020 in license and sphinx conf

* changelog for energy_from_power, normalize_with_x (#172)

* changelog for energy_from_power, normalize_with_x

* revise changelog

* migrate to pvlib>=0.7.0 (#170)

* migrate to pvlib >=0.7.0

* update SAPM normalization docstrings to mention racking_model and module_type

* update sapm normalization test to specify module_type

* bump docs pvlib requirement to 0.7.1

* Update rdtools/normalization.py

Co-Authored-By: Cliff Hansen <cwhanse@sandia.gov>

* don't use keyword names for sapm_celltemp

Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>

* better GitHub links in documentation (#156)

* tentative fix

* fix slash

* include branch logic based on RTD version

* small improvement

* omit GH link element if not building development or master

* change github link text to reflect the destination branch

* link changelog page to changelog GH folder instead of the template rst file

* bugfix

* Generalized model normalizaiton (#173)

* Add normalize_with_expected_power() and refactor normalization module

* make minor adjustments to normalize_with_expected_power

* Add tests for normalize_with_expected_power

* update changelog

* update docstring

Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

* Change test file name

* code formatting fixes

* simplify conditional

* return energy and insolation on same index

* minor changelog update

* add new function to __init__.py and api.rst

Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

* Include emacs auto-save and backup files in gitignore (#146)

* Add example notebook using the NREL PVDAQ #4 system (#171)

* Create degradation_and_soiling_example_sanyo.ipynb

* various typos

* change weather columns based on inspection; rename nz_mask to normalized_mask; improve soiling analysis

* bump pandas, pvlib, and statsmodels versions in requirements.txt

* add discussion; rerun notebook

* rerun original notebook with new requirements to remove warnings

* update pvlib links in notebooks to fix rendering on RTD

* use sanyo example for the RTD page

* changelog

* clean up labeling

* Notebook updates

-Update the notebook to use a more pronounced soiling signal
-Comment updates
-use normalized_filter()
-Use default behavior of infer_freq() and energy_from_power()

* Update docs/degradation_and_soiling_example_pvdaq_4.ipynb

Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

* Update docs/degradation_and_soiling_example_pvdaq_4.ipynb

Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

* Update notebook to use new normalize_with_expected_power()

* limit notebook line length to 90 characters

* Update notebook with data location on datahub

* add pickle to gitignore

* Cache data and fix typo

Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com>

* add the logo to documentation home page (#174)

* add logo to index.rst

* Update logo_horizontal_highres.png

* Add logo to readme (#178)

* Add logo to readme

* Shrink logo

* update DKASC link (#180)

* SRR precipitation changes (#176)

* remove "clean_wo_precip", add "clean_criterion"

* loop to cumulative sum

* missed some changes

* add min_interval_length parameter to SRR

* clean up code formatting

* Move clean event outage and consecutive day logic to after cleaning events are established

* Add precip and shift options for clean_criterion

* Add precip_threshold

* Update precipitation cleaning detection logic

Use a three day window only for precip_and_shift case

* Update docstrings about when precipitation is used

* update soiling test for precip behavior

* rerun notebooks

pvdaq4 notebook unaffected

* break a docstring line

* update the changelog

* docstring typo

* change log typo fix

Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>

* Review updates

* splat kwargs in test

* remove random_seed

Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com>

* Change interpolate max_timedelta to 2x median, add warning (#182)

* change threshold, add warning, update tests

* fix how exclusion warning is calculated, rerun notebooks

* changelog

* add license file to manifest (#183)

* Overview update (#186)

* Overview update

I suggest slightly rephrasing the overview.

* Add change to readme

* Add GitHub templates and Code of Conduct (#184)

* Create pull_request_template.md

* add default github templates

* improvements from review

* Comply with pv-terms (#185)

* first pass at pvterms compat

* change back to `gamma_pdc`

* docstring update power->energy

* Capitalize Celsius

* dc_power to power_dc (local variable)

* modeled_irrad to irrad_sim

* 1time_series` to `power`

* add simultated to docstring

* update test with gamma_pdc

* Fix failing tests Possibly related to a pandas update rather than anything in RdTools

* Update soiling attributes

* pvwatts kwargs docstring update

* Updated example notebooks: 'tempco' = 'gamma_pdc'; 'wind' = 'wind_speed'

* Pandas > 1.0 requires explicit registering of matplotlib converters.

* update normalize_with_pvwatts and clearsky_pvwatts_kws keywords. Pref-> power_dc_rated.

* Add sphinx requirements to requirements.txt

* Update API example in index.rst.  Add #185 updates to whatsnew

* Add specific kwarg values that have changed

Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com>
Co-authored-by: cdeline <chris.deline@nrel.gov>

* Update changelog with 2.0b0 release date

Co-authored-by: Michael Deceglie <michael.deceglie@nrel.gov>
Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com>
Co-authored-by: Kevin Anderson <57452607+kanderso-nrel@users.noreply.github.com>
Co-authored-by: Kevin Anderson <kevin.anderson@nrel.gov>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
Co-authored-by: Will Vining <wfvining@gmail.com>
@kandersolar kandersolar mentioned this pull request Sep 3, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants