Skip to content

migrate to pvlib>=0.7.0 #170

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 7 commits into from
Apr 29, 2020
Merged

migrate to pvlib>=0.7.0 #170

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
559cb13
migrate to pvlib >=0.7.0
kandersolar Apr 22, 2020
4fbb05a
update SAPM normalization docstrings to mention racking_model and mod…
kandersolar Apr 23, 2020
c9a39d2
update sapm normalization test to specify module_type
kandersolar Apr 23, 2020
c04f16e
bump docs pvlib requirement to 0.7.1
kandersolar Apr 23, 2020
4747588
Update rdtools/normalization.py
kandersolar Apr 23, 2020
baed513
don't use keyword names for sapm_celltemp
kandersolar Apr 23, 2020
08f16dc
Merge branch 'pvlib_compat' of https://github.com/NREL/rdtools into p…
kandersolar Apr 23, 2020
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
55 changes: 38 additions & 17 deletions docs/degradation_and_soiling_example.ipynb
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions docs/sphinx/source/changelog/v2.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Documentation
Requirements
------------
* Drop support for Python 2.7, minimum supported version is now 3.6 (:pull:`135`).
* Increase minimum pvlib version to 0.7.0.
* Update requirements.txt and notebook_requirements.txt to avoid conflicting specifications. Taken together,
they represent the complete environment for the notebook example (:pull:`164`).

Expand Down
19 changes: 11 additions & 8 deletions rdtools/normalization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ def sapm_dc_power(pvlib_pvsystem, met_data):
----------
pvlib_pvsystem : pvlib-python LocalizedPVSystem object
Object contains orientation, geographic coordinates, equipment
constants (including DC rated power in watts).
constants (including DC rated power in watts). The object must also
specify either the `temperature_model_parameters` attribute or both
`racking_model` and `module_type` attributes to infer the temperature model parameters.
met_data : pd.DataFrame
Measured irradiance components, ambient temperature, and wind speed.
Expected met_data DataFrame column names:
Expand Down Expand Up @@ -184,17 +186,16 @@ def sapm_dc_power(pvlib_pvsystem, met_data):
poa_diffuse=total_irradiance['poa_diffuse'],
airmass_absolute=airmass_absolute,
aoi=aoi,
module=pvlib_pvsystem.module,
reference_irradiance=1)
module=pvlib_pvsystem.module)

temp_cell = pvlib_pvsystem\
.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

@cwhanse cwhanse Apr 23, 2020

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

@kandersolar kandersolar Apr 23, 2020

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 •
edited
Loading

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

@kandersolar kandersolar Apr 23, 2020

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

@mdeceglie mdeceglie Apr 23, 2020

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

@mdeceglie mdeceglie Apr 23, 2020

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)

wind_speed=met_data['Wind Speed'],
temp_air=met_data['Temperature'])

dc_power = pvlib_pvsystem\
.pvwatts_dc(g_poa_effective=effective_poa,
temp_cell=temp_cell['temp_cell'])
temp_cell=temp_cell)

return dc_power, effective_poa

Expand All @@ -221,7 +222,9 @@ def normalize_with_sapm(energy, sapm_kws):
---------------
pvlib_pvsystem : pvlib-python LocalizedPVSystem object
Object contains orientation, geographic coordinates, equipment
constants.
constants (including DC rated power in watts). The object must also
specify either the `temperature_model_parameters` attribute or both
`racking_model` and `module_type` to infer the model parameters.
met_data : pd.DataFrame
Measured met_data, ambient temperature, and wind speed. Expected
column names are ['DNI', 'GHI', 'DHI', 'Temperature', 'Wind Speed']
Expand Down
3 changes: 2 additions & 1 deletion rdtools/test/normalization_sapm_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ def setUp(self):
surface_azimuth=180,
module=module,
module_parameters=module_parameters,
racking_model='insulated_back_polymerback',
racking_model='insulated_back',
module_type='glass_polymer',
modules_per_string=6)

# define dummy energy data
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ matplotlib==3.1.2
numpy==1.17.3
pandas==0.25.2
patsy==0.5.1
pvlib==0.6.1
pvlib==0.7.1
pyparsing==2.4.7
python-dateutil==2.8.1
pytz==2019.3
Expand Down
4 changes: 2 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
'statsmodels >= 0.8.0',
'scipy >= 0.19.1',
'h5py >= 2.7.1',
'pvlib >= 0.6.0, <0.7.0',
'pvlib >= 0.7.0, <0.8.0',
]

EXTRAS_REQUIRE = {
Expand All @@ -51,7 +51,7 @@
'nbsphinx==0.4.3',
'nbsphinx-link==1.3.0',
'pandas==0.23.0',
'pvlib==0.6.1',
'pvlib==0.7.1',
'sphinx_rtd_theme==0.4.3',
'ipython',
],
Expand Down