Skip to content

add consistency tests to ModelChain.dc_model #548

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 13 commits into from
Aug 31, 2018
3 changes: 3 additions & 0 deletions docs/sphinx/source/whatsnew/v0.6.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ API Changes
improved results under other conditions. (:issue:`435`)
* Add min_cos_zenith, max_zenith keyword arguments to disc, dirint, and
dirindex functions. (:issue:`311`, :issue:`396`)
* Method ModelChain.infer_dc_model now returns a tuple (function handle, model name string)
instead of only the function handle (:issue:`417`)


Enhancements
Expand Down Expand Up @@ -101,6 +103,7 @@ Enhancements
* Add irradiance.gti_dirint function. (:issue:`396`)
* Add irradiance.clearness_index function. (:issue:`396`)
* Add irradiance.clearness_index_zenith_independent function. (:issue:`396`)
* Add checking for consistency between module_parameters and dc_model. (:issue:`417`)


Bug fixes
Expand Down
35 changes: 24 additions & 11 deletions pvlib/modelchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from pvlib import (solarposition, pvsystem, clearsky, atmosphere, tools)
from pvlib.tracking import SingleAxisTracker
import pvlib.irradiance # avoid name conflict with full import
from pvlib.pvsystem import DC_MODEL_PARAMS


def basic_chain(times, latitude, longitude,
Expand Down Expand Up @@ -360,16 +361,28 @@ def dc_model(self):

@dc_model.setter
def dc_model(self, model):
# guess at model if None
if model is None:
self._dc_model = self.infer_dc_model()
elif isinstance(model, str):
self._dc_model, model = self.infer_dc_model()

# Set model and validate parameters
if isinstance(model, str):
model = model.lower()
if model == 'sapm':
self._dc_model = self.sapm
elif model == 'singlediode':
self._dc_model = self.singlediode
elif model == 'pvwatts':
self._dc_model = self.pvwatts_dc
if model in DC_MODEL_PARAMS.keys():
# validate module parameters
missing_params = DC_MODEL_PARAMS[model] - \
set(self.system.module_parameters.keys())
if missing_params: # some parameters are not in module.keys()
raise ValueError(model + ' selected for the DC model but '
'one or more required parameters '
'are missing : ' +
str(missing_params))
if model == 'sapm':
self._dc_model = self.sapm
elif model == 'singlediode':
self._dc_model = self.singlediode
elif model == 'pvwatts':
self._dc_model = self.pvwatts_dc
else:
raise ValueError(model + ' is not a valid DC power model')
else:
Expand All @@ -378,11 +391,11 @@ def dc_model(self, model):
def infer_dc_model(self):
params = set(self.system.module_parameters.keys())
if set(['A0', 'A1', 'C7']) <= params:
return self.sapm
return self.sapm, 'sapm'
elif set(['a_ref', 'I_L_ref', 'I_o_ref', 'R_sh_ref', 'R_s']) <= params:
return self.singlediode
return self.singlediode, 'singlediode'
elif set(['pdc0', 'gamma_pdc']) <= params:
return self.pvwatts_dc
return self.pvwatts_dc, 'pvwatts'
else:
raise ValueError('could not infer DC model from '
'system.module_parameters')
Expand Down
16 changes: 16 additions & 0 deletions pvlib/pvsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,22 @@
import pvlib # use pvlib.singlediode to avoid clash with local method


# a dict of required parameter names for each DC power model

DC_MODEL_PARAMS = {'sapm' :
set(['A0', 'A1', 'A2', 'A3', 'A4', 'B0', 'B1', 'B2', 'B3',
'B4', 'B5', 'C0', 'C1', 'C2', 'C3', 'C4', 'C5', 'C6',
'C7', 'Isco', 'Impo', 'Aisc', 'Aimp', 'Bvoco',
'Mbvoc', 'Bvmpo', 'Mbvmp', 'N', 'Cells_in_Series',
'IXO', 'IXXO', 'FD']),
'singlediode' :
set(['alpha_sc', 'a_ref', 'I_L_ref', 'I_o_ref',
'R_sh_ref', 'R_s']),
'pvwatts' :
set(['pdc0', 'gamma_pdc'])
}


# not sure if this belongs in the pvsystem module.
# maybe something more like core.py? It may eventually grow to
# import a lot more functionality from other modules.
Expand Down
33 changes: 30 additions & 3 deletions pvlib/test/test_modelchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
@pytest.fixture
def system(sam_data):
modules = sam_data['sandiamod']
module_parameters = modules['Canadian_Solar_CS5P_220M___2009_'].copy()
module = 'Canadian_Solar_CS5P_220M___2009_'
module_parameters = modules[module].copy()
inverters = sam_data['cecinverter']
inverter = inverters['ABB__MICRO_0_25_I_OUTD_US_208_208V__CEC_2014_'].copy()
system = PVSystem(surface_tilt=32.2, surface_azimuth=180,
module=module,
module_parameters=module_parameters,
inverter_parameters=inverter)
return system
Expand All @@ -37,13 +39,15 @@ def system(sam_data):
@pytest.fixture
def cec_dc_snl_ac_system(sam_data):
modules = sam_data['cecmod']
module_parameters = modules['Canadian_Solar_CS5P_220M'].copy()
module = 'Canadian_Solar_CS5P_220M'
module_parameters = modules[module].copy()
module_parameters['b'] = 0.05
module_parameters['EgRef'] = 1.121
module_parameters['dEgdT'] = -0.0002677
inverters = sam_data['cecinverter']
inverter = inverters['ABB__MICRO_0_25_I_OUTD_US_208_208V__CEC_2014_'].copy()
system = PVSystem(surface_tilt=32.2, surface_azimuth=180,
module=module,
module_parameters=module_parameters,
inverter_parameters=inverter)
return system
Expand All @@ -52,13 +56,15 @@ def cec_dc_snl_ac_system(sam_data):
@pytest.fixture
def cec_dc_adr_ac_system(sam_data):
modules = sam_data['cecmod']
module_parameters = modules['Canadian_Solar_CS5P_220M'].copy()
module = 'Canadian_Solar_CS5P_220M'
module_parameters = modules[module].copy()
module_parameters['b'] = 0.05
module_parameters['EgRef'] = 1.121
module_parameters['dEgdT'] = -0.0002677
inverters = sam_data['adrinverter']
inverter = inverters['Zigor__Sunzet_3_TL_US_240V__CEC_2011_'].copy()
system = PVSystem(surface_tilt=32.2, surface_azimuth=180,
module=module,
module_parameters=module_parameters,
inverter_parameters=inverter)
return system
Expand Down Expand Up @@ -365,6 +371,27 @@ def test_losses_models_no_loss(pvwatts_dc_pvwatts_ac_system, location, weather,
assert mc.losses == 1


def test_invalid_dc_model_params(system, cec_dc_snl_ac_system,
pvwatts_dc_pvwatts_ac_system, location):
kwargs = {'dc_model': 'sapm', 'ac_model': 'snlinverter',
'aoi_model': 'no_loss', 'spectral_model': 'no_loss',
'temp_model': 'sapm', 'losses_model': 'no_loss'}
system.module_parameters.pop('A0') # remove a parameter
with pytest.raises(ValueError):
mc = ModelChain(system, location, **kwargs)

kwargs['dc_model'] = 'singlediode'
cec_dc_snl_ac_system.module_parameters.pop('a_ref') # remove a parameter
with pytest.raises(ValueError):
mc = ModelChain(cec_dc_snl_ac_system, location, **kwargs)

kwargs['dc_model'] = 'pvwatts'
kwargs['ac_model'] = 'pvwatts'
pvwatts_dc_pvwatts_ac_system.module_parameters.pop('pdc0')
with pytest.raises(ValueError):
mc = ModelChain(pvwatts_dc_pvwatts_ac_system, location, **kwargs)


@pytest.mark.parametrize('model', [
'dc_model', 'ac_model', 'aoi_model', 'spectral_model', 'losses_model',
'temp_model', 'losses_model'
Expand Down