Skip to content

Upgrade bifacial functions to work with pvfactors v1.4.1 #934

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
Jun 18, 2020
2 changes: 1 addition & 1 deletion ci/requirements-py35.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ dependencies:
- siphon # conda-forge
- pip:
- nrel-pysam>=2.0
- pvfactors==1.0.1
- pvfactors==1.4.1
- pytest-rerunfailures # conda version is >3.6
- pytest-remotedata # needs > 0.3.1
2 changes: 1 addition & 1 deletion ci/requirements-py36.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ dependencies:
- siphon # conda-forge
- pip:
- nrel-pysam>=2.0
- pvfactors==1.0.1
- pvfactors==1.4.1
2 changes: 1 addition & 1 deletion ci/requirements-py37.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ dependencies:
- siphon # conda-forge
- pip:
- nrel-pysam>=2.0
- pvfactors==1.0.1
- pvfactors==1.4.1
2 changes: 1 addition & 1 deletion ci/requirements-py38.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ dependencies:
- siphon # conda-forge
- pip:
- nrel-pysam>=2.0
- pvfactors==1.0.1
- pvfactors==1.4.1
4 changes: 4 additions & 0 deletions docs/sphinx/source/whatsnew/v0.8.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ v0.8.0 (Month day, year)

API Changes
~~~~~~~~~~~
* Removed ``run_parallel_calculations`` and ``n_workers_for_parallel_calcs``
from :py:func:`pvlib.bifacial.pvfactors_timeseries` inputs (:issue:`902`)(:pull:`934`)

Enhancements
~~~~~~~~~~~~
* Update :func:`~pvlib.bifacial.pvfactors_timeseries` to run with ``pvfactors`` v1.4.1 (:issue:`902`)(:pull:`934`)

Bug fixes
~~~~~~~~~
Expand Down Expand Up @@ -36,3 +39,4 @@ Contributors
* Kevin Anderson (:ghuser:`kanderso-nrel`)
* Mark Mikofski (:ghuser:`mikofski`)
* Joshua S. Stein (:ghuser:`jsstein`)
* Marc A. Anoma (:ghuser:`anomam`)
127 changes: 45 additions & 82 deletions pvlib/bifacial.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@

def pvfactors_timeseries(
solar_azimuth, solar_zenith, surface_azimuth, surface_tilt,
axis_azimuth,
timestamps, dni, dhi, gcr, pvrow_height, pvrow_width, albedo,
n_pvrows=3, index_observed_pvrow=1,
axis_azimuth, timestamps, dni, dhi, gcr, pvrow_height, pvrow_width,
albedo, n_pvrows=3, index_observed_pvrow=1,
rho_front_pvrow=0.03, rho_back_pvrow=0.05,
horizon_band_angle=15.,
run_parallel_calculations=True, n_workers_for_parallel_calcs=2):
horizon_band_angle=15.):
"""
Calculate front and back surface plane-of-array irradiance on
a fixed tilt or single-axis tracker PV array configuration, and using
Expand Down Expand Up @@ -62,127 +60,92 @@ def pvfactors_timeseries(
Back surface reflectivity of PV rows
horizon_band_angle: float, default 15
Elevation angle of the sky dome's diffuse horizon band (deg)
run_parallel_calculations: bool, default True
pvfactors is capable of using multiprocessing. Use this flag to decide
to run calculations in parallel (recommended) or not.
n_workers_for_parallel_calcs: int, default 2
Number of workers to use in the case of parallel calculations. The
'-1' value will lead to using a value equal to the number
of CPU's on the machine running the model.

Returns
-------
front_poa_irradiance: numeric
poa_front: numeric
Calculated incident irradiance on the front surface of the PV modules
(W/m2)
back_poa_irradiance: numeric
poa_back: numeric
Calculated incident irradiance on the back surface of the PV modules
(W/m2)
df_registries: pandas DataFrame
DataFrame containing detailed outputs of the simulation; for
instance the shapely geometries, the irradiance components incident on
all surfaces of the PV array (for all timestamps), etc.
In the pvfactors documentation, this is refered to as the "surface
registry".
poa_front_absorbed: numeric
Calculated absorbed irradiance on the front surface of the PV modules
(W/m2), after AOI losses
poa_back_absorbed: numeric
Calculated absorbed irradiance on the back surface of the PV modules
(W/m2), after AOI losses
Comment on lines -75 to +77
Copy link
Member

Choose a reason for hiding this comment

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

If I understand the diff correctly, this change to the output will break user code. Ok with me but we should note it in the API Changes section. I guess there's not much point in deprecating the parallel kwargs if we're just changing the return type, so maybe best to just make users deal with the pain in the next release. I'm guessing that users of the bifacial module are relatively tolerant of api changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @wholmgren :

  • I could revert the output change and return some kind of placeholder for df_registries (pvfactors v1.4.1 does not use registries anymore). Or I can just update the API changes section. Just let me know what you prefer!
  • I don't mind removing the deprecation warnings as well

Copy link
Member

Choose a reason for hiding this comment

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

In this case I think we're better off immediately conforming to the pvfactors 1.4 API, provided the API change is documented. Any objections?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @wholmgren , sorry for the delay. I removed the deprecated inputs and warnings, and updated the API changes section of the newest whatsnew file. Please let me know if you need any other fixes


References
----------
.. [1] Anoma, Marc Abou, et al. "View Factor Model and Validation for
Bifacial PV and Diffuse Shade on Single-Axis Trackers." 44th IEEE
Photovoltaic Specialist Conference. 2017.
"""

# Convert pandas Series inputs (and some lists) to numpy arrays
if isinstance(solar_azimuth, pd.Series):
solar_azimuth = solar_azimuth.values
elif isinstance(solar_azimuth, list):
solar_azimuth = np.array(solar_azimuth)
if isinstance(solar_zenith, pd.Series):
solar_zenith = solar_zenith.values
elif isinstance(solar_zenith, list):
solar_zenith = np.array(solar_zenith)
if isinstance(surface_azimuth, pd.Series):
surface_azimuth = surface_azimuth.values
elif isinstance(surface_azimuth, list):
surface_azimuth = np.array(surface_azimuth)
if isinstance(surface_tilt, pd.Series):
surface_tilt = surface_tilt.values
elif isinstance(surface_tilt, list):
surface_tilt = np.array(surface_tilt)
if isinstance(dni, pd.Series):
dni = dni.values
elif isinstance(dni, list):
dni = np.array(dni)
if isinstance(dhi, pd.Series):
dhi = dhi.values
if isinstance(solar_azimuth, list):
solar_azimuth = np.array(solar_azimuth)
elif isinstance(dhi, list):
dhi = np.array(dhi)

# Import pvfactors functions for timeseries calculations.
from pvfactors.run import (run_timeseries_engine,
run_parallel_engine)
from pvfactors.run import run_timeseries_engine

# Build up pv array configuration parameters
pvarray_parameters = {
'n_pvrows': n_pvrows,
'axis_azimuth': axis_azimuth,
'pvrow_height': pvrow_height,
'pvrow_width': pvrow_width,
'gcr': gcr,
'rho_front_pvrow': rho_front_pvrow,
'rho_back_pvrow': rho_back_pvrow,
'gcr': gcr
}

irradiance_model_params = {
'rho_front': rho_front_pvrow,
'rho_back': rho_back_pvrow,
'horizon_band_angle': horizon_band_angle
}

# Run pvfactors calculations: either in parallel or serially
if run_parallel_calculations:
report = run_parallel_engine(
PVFactorsReportBuilder, pvarray_parameters,
timestamps, dni, dhi,
solar_zenith, solar_azimuth,
surface_tilt, surface_azimuth,
albedo, n_processes=n_workers_for_parallel_calcs)
else:
report = run_timeseries_engine(
PVFactorsReportBuilder.build, pvarray_parameters,
timestamps, dni, dhi,
solar_zenith, solar_azimuth,
surface_tilt, surface_azimuth,
albedo)
# Create report function
def fn_build_report(pvarray):
return {'total_inc_back': pvarray.ts_pvrows[index_observed_pvrow]
.back.get_param_weighted('qinc'),
'total_inc_front': pvarray.ts_pvrows[index_observed_pvrow]
.front.get_param_weighted('qinc'),
'total_abs_back': pvarray.ts_pvrows[index_observed_pvrow]
.back.get_param_weighted('qabs'),
'total_abs_front': pvarray.ts_pvrows[index_observed_pvrow]
.front.get_param_weighted('qabs')}

# Run pvfactors calculations
report = run_timeseries_engine(
fn_build_report, pvarray_parameters,
timestamps, dni, dhi, solar_zenith, solar_azimuth,
surface_tilt, surface_azimuth, albedo,
irradiance_model_params=irradiance_model_params)

# Turn report into dataframe
df_report = pd.DataFrame(report, index=timestamps)

return df_report.total_inc_front, df_report.total_inc_back


class PVFactorsReportBuilder(object):
"""In pvfactors, a class is required to build reports when running
calculations with multiprocessing because of python constraints"""

@staticmethod
def build(report, pvarray):
"""Reports will have total incident irradiance on front and
back surface of center pvrow (index=1)"""
# Initialize the report as a dictionary
if report is None:
report = {'total_inc_back': [], 'total_inc_front': []}
# Add elements to the report
if pvarray is not None:
pvrow = pvarray.pvrows[1] # use center pvrow
report['total_inc_back'].append(
pvrow.back.get_param_weighted('qinc'))
report['total_inc_front'].append(
pvrow.front.get_param_weighted('qinc'))
else:
# No calculation is performed when the sun is down
report['total_inc_back'].append(np.nan)
report['total_inc_front'].append(np.nan)

return report

@staticmethod
def merge(reports):
"""Works for dictionary reports. Merges the reports list of
dictionaries in a single dictionary. The list of the first
dictionary are extended by those of all subsequent lists."""
report = reports[0]
keys_report = list(report.keys())
for other_report in reports[1:]: # loop won't run if len(reports) < 2
for key in keys_report:
report[key] += other_report[key]
return report
return (df_report.total_inc_front, df_report.total_inc_back,
df_report.total_abs_front, df_report.total_abs_back)
75 changes: 13 additions & 62 deletions pvlib/tests/test_bifacial.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import pandas as pd
import numpy as np
from datetime import datetime
from pvlib.bifacial import pvfactors_timeseries, PVFactorsReportBuilder
from pvlib.bifacial import pvfactors_timeseries
from conftest import requires_pvfactors
import pytest


@requires_pvfactors
@pytest.mark.parametrize('run_parallel_calculations',
[False, True])
def test_pvfactors_timeseries(run_parallel_calculations):
def test_pvfactors_timeseries():
""" Test that pvfactors is functional, using the TLDR section inputs of the
package github repo README.md file:
https://github.com/SunPower/pvfactors/blob/master/README.md#tldr---quick-start"""
Expand Down Expand Up @@ -39,29 +36,25 @@ def test_pvfactors_timeseries(run_parallel_calculations):
expected_ipoa_front = pd.Series([1034.95474708997, 795.4423259036623],
index=timestamps,
name=('total_inc_front'))
expected_ipoa_back = pd.Series([91.88707460262768, 78.05831585685215],
expected_ipoa_back = pd.Series([92.12563846416197, 78.05831585685098],
index=timestamps,
name=('total_inc_back'))

# Run calculation
ipoa_front, ipoa_back = pvfactors_timeseries(
ipoa_inc_front, ipoa_inc_back, _, _ = pvfactors_timeseries(
solar_azimuth, solar_zenith, surface_azimuth, surface_tilt,
axis_azimuth,
timestamps, dni, dhi, gcr, pvrow_height, pvrow_width, albedo,
n_pvrows=n_pvrows, index_observed_pvrow=index_observed_pvrow,
rho_front_pvrow=rho_front_pvrow, rho_back_pvrow=rho_back_pvrow,
horizon_band_angle=horizon_band_angle,
run_parallel_calculations=run_parallel_calculations,
n_workers_for_parallel_calcs=-1)
horizon_band_angle=horizon_band_angle)

pd.testing.assert_series_equal(ipoa_front, expected_ipoa_front)
pd.testing.assert_series_equal(ipoa_back, expected_ipoa_back)
pd.testing.assert_series_equal(ipoa_inc_front, expected_ipoa_front)
pd.testing.assert_series_equal(ipoa_inc_back, expected_ipoa_back)


@requires_pvfactors
@pytest.mark.parametrize('run_parallel_calculations',
[False, True])
def test_pvfactors_timeseries_pandas_inputs(run_parallel_calculations):
def test_pvfactors_timeseries_pandas_inputs():
""" Test that pvfactors is functional, using the TLDR section inputs of the
package github repo README.md file, but converted to pandas Series:
https://github.com/SunPower/pvfactors/blob/master/README.md#tldr---quick-start"""
Expand Down Expand Up @@ -91,60 +84,18 @@ def test_pvfactors_timeseries_pandas_inputs(run_parallel_calculations):
expected_ipoa_front = pd.Series([1034.95474708997, 795.4423259036623],
index=timestamps,
name=('total_inc_front'))
expected_ipoa_back = pd.Series([91.88707460262768, 78.05831585685215],
expected_ipoa_back = pd.Series([92.12563846416197, 78.05831585685098],
index=timestamps,
name=('total_inc_back'))

# Run calculation
ipoa_front, ipoa_back = pvfactors_timeseries(
ipoa_inc_front, ipoa_inc_back, _, _ = pvfactors_timeseries(
solar_azimuth, solar_zenith, surface_azimuth, surface_tilt,
axis_azimuth,
timestamps, dni, dhi, gcr, pvrow_height, pvrow_width, albedo,
n_pvrows=n_pvrows, index_observed_pvrow=index_observed_pvrow,
rho_front_pvrow=rho_front_pvrow, rho_back_pvrow=rho_back_pvrow,
horizon_band_angle=horizon_band_angle,
run_parallel_calculations=run_parallel_calculations,
n_workers_for_parallel_calcs=-1)
horizon_band_angle=horizon_band_angle)

pd.testing.assert_series_equal(ipoa_front, expected_ipoa_front)
pd.testing.assert_series_equal(ipoa_back, expected_ipoa_back)


def test_build_1():
"""Test that build correctly instantiates a dictionary, when passed a Nones
for the report and pvarray arguments.
"""
report = None
pvarray = None
expected = {'total_inc_back': [np.nan], 'total_inc_front': [np.nan]}
assert expected == PVFactorsReportBuilder.build(report, pvarray)


def test_merge_1():
"""Test that merge correctly returns the first element of the reports
argument when there is only dictionary in reports.
"""
test_dict = {'total_inc_back': [1, 2, 3], 'total_inc_front': [4, 5, 6]}
reports = [test_dict]
assert test_dict == PVFactorsReportBuilder.merge(reports)


def test_merge_2():
"""Test that merge correctly combines two dictionary reports.
"""
test_dict_1 = {'total_inc_back': [1, 2], 'total_inc_front': [4, 5]}
test_dict_2 = {'total_inc_back': [3], 'total_inc_front': [6]}
expected = {'total_inc_back': [1, 2, 3], 'total_inc_front': [4, 5, 6]}
reports = [test_dict_1, test_dict_2]
assert expected == PVFactorsReportBuilder.merge(reports)


def test_merge_3():
"""Test that merge correctly combines three dictionary reports.
"""
test_dict_1 = {'total_inc_back': [1], 'total_inc_front': [4]}
test_dict_2 = {'total_inc_back': [2], 'total_inc_front': [5]}
test_dict_3 = {'total_inc_back': [3], 'total_inc_front': [6]}
expected = {'total_inc_back': [1, 2, 3], 'total_inc_front': [4, 5, 6]}
reports = [test_dict_1, test_dict_2, test_dict_3]
assert expected == PVFactorsReportBuilder.merge(reports)
pd.testing.assert_series_equal(ipoa_inc_front, expected_ipoa_front)
pd.testing.assert_series_equal(ipoa_inc_back, expected_ipoa_back)