Skip to content

Commit abe8bf1

Browse files
authored
add consistency tests to ModelChain.dc_model (#548)
* add consistency tests to ModelChain.dc_model * Add self argument * Move DC_MODEL_PARAMS to pvsystem, streamline validation * Fix key for DC_MODEL_PARAMS * Fix typo in IXO, IXXO * Change logic in dc_model, correct pvwatts keyword * More adjustment to pvwatts_dc vs pvwatts keyword * Add handling of invalid model name string * Add test for invalid dc model parameters * Fix typo in cec_dc_adr_ac_system * Add to whatsnew.rst * Cleanup, remove copy from test * Delete comment from infer_dc_model
1 parent e3faaad commit abe8bf1

File tree

4 files changed

+73
-14
lines changed

4 files changed

+73
-14
lines changed

docs/sphinx/source/whatsnew/v0.6.0.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ API Changes
5151
improved results under other conditions. (:issue:`435`)
5252
* Add min_cos_zenith, max_zenith keyword arguments to disc, dirint, and
5353
dirindex functions. (:issue:`311`, :issue:`396`)
54+
* Method ModelChain.infer_dc_model now returns a tuple (function handle, model name string)
55+
instead of only the function handle (:issue:`417`)
5456

5557

5658
Enhancements
@@ -101,6 +103,7 @@ Enhancements
101103
* Add irradiance.gti_dirint function. (:issue:`396`)
102104
* Add irradiance.clearness_index function. (:issue:`396`)
103105
* Add irradiance.clearness_index_zenith_independent function. (:issue:`396`)
106+
* Add checking for consistency between module_parameters and dc_model. (:issue:`417`)
104107

105108

106109
Bug fixes

pvlib/modelchain.py

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from pvlib import (solarposition, pvsystem, clearsky, atmosphere, tools)
1414
from pvlib.tracking import SingleAxisTracker
1515
import pvlib.irradiance # avoid name conflict with full import
16+
from pvlib.pvsystem import DC_MODEL_PARAMS
1617

1718

1819
def basic_chain(times, latitude, longitude,
@@ -360,16 +361,28 @@ def dc_model(self):
360361

361362
@dc_model.setter
362363
def dc_model(self, model):
364+
# guess at model if None
363365
if model is None:
364-
self._dc_model = self.infer_dc_model()
365-
elif isinstance(model, str):
366+
self._dc_model, model = self.infer_dc_model()
367+
368+
# Set model and validate parameters
369+
if isinstance(model, str):
366370
model = model.lower()
367-
if model == 'sapm':
368-
self._dc_model = self.sapm
369-
elif model == 'singlediode':
370-
self._dc_model = self.singlediode
371-
elif model == 'pvwatts':
372-
self._dc_model = self.pvwatts_dc
371+
if model in DC_MODEL_PARAMS.keys():
372+
# validate module parameters
373+
missing_params = DC_MODEL_PARAMS[model] - \
374+
set(self.system.module_parameters.keys())
375+
if missing_params: # some parameters are not in module.keys()
376+
raise ValueError(model + ' selected for the DC model but '
377+
'one or more required parameters '
378+
'are missing : ' +
379+
str(missing_params))
380+
if model == 'sapm':
381+
self._dc_model = self.sapm
382+
elif model == 'singlediode':
383+
self._dc_model = self.singlediode
384+
elif model == 'pvwatts':
385+
self._dc_model = self.pvwatts_dc
373386
else:
374387
raise ValueError(model + ' is not a valid DC power model')
375388
else:
@@ -378,11 +391,11 @@ def dc_model(self, model):
378391
def infer_dc_model(self):
379392
params = set(self.system.module_parameters.keys())
380393
if set(['A0', 'A1', 'C7']) <= params:
381-
return self.sapm
394+
return self.sapm, 'sapm'
382395
elif set(['a_ref', 'I_L_ref', 'I_o_ref', 'R_sh_ref', 'R_s']) <= params:
383-
return self.singlediode
396+
return self.singlediode, 'singlediode'
384397
elif set(['pdc0', 'gamma_pdc']) <= params:
385-
return self.pvwatts_dc
398+
return self.pvwatts_dc, 'pvwatts'
386399
else:
387400
raise ValueError('could not infer DC model from '
388401
'system.module_parameters')

pvlib/pvsystem.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,22 @@
2323
import pvlib # use pvlib.singlediode to avoid clash with local method
2424

2525

26+
# a dict of required parameter names for each DC power model
27+
28+
DC_MODEL_PARAMS = {'sapm' :
29+
set(['A0', 'A1', 'A2', 'A3', 'A4', 'B0', 'B1', 'B2', 'B3',
30+
'B4', 'B5', 'C0', 'C1', 'C2', 'C3', 'C4', 'C5', 'C6',
31+
'C7', 'Isco', 'Impo', 'Aisc', 'Aimp', 'Bvoco',
32+
'Mbvoc', 'Bvmpo', 'Mbvmp', 'N', 'Cells_in_Series',
33+
'IXO', 'IXXO', 'FD']),
34+
'singlediode' :
35+
set(['alpha_sc', 'a_ref', 'I_L_ref', 'I_o_ref',
36+
'R_sh_ref', 'R_s']),
37+
'pvwatts' :
38+
set(['pdc0', 'gamma_pdc'])
39+
}
40+
41+
2642
# not sure if this belongs in the pvsystem module.
2743
# maybe something more like core.py? It may eventually grow to
2844
# import a lot more functionality from other modules.

pvlib/test/test_modelchain.py

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
@pytest.fixture
2626
def system(sam_data):
2727
modules = sam_data['sandiamod']
28-
module_parameters = modules['Canadian_Solar_CS5P_220M___2009_'].copy()
28+
module = 'Canadian_Solar_CS5P_220M___2009_'
29+
module_parameters = modules[module].copy()
2930
inverters = sam_data['cecinverter']
3031
inverter = inverters['ABB__MICRO_0_25_I_OUTD_US_208_208V__CEC_2014_'].copy()
3132
system = PVSystem(surface_tilt=32.2, surface_azimuth=180,
33+
module=module,
3234
module_parameters=module_parameters,
3335
inverter_parameters=inverter)
3436
return system
@@ -37,13 +39,15 @@ def system(sam_data):
3739
@pytest.fixture
3840
def cec_dc_snl_ac_system(sam_data):
3941
modules = sam_data['cecmod']
40-
module_parameters = modules['Canadian_Solar_CS5P_220M'].copy()
42+
module = 'Canadian_Solar_CS5P_220M'
43+
module_parameters = modules[module].copy()
4144
module_parameters['b'] = 0.05
4245
module_parameters['EgRef'] = 1.121
4346
module_parameters['dEgdT'] = -0.0002677
4447
inverters = sam_data['cecinverter']
4548
inverter = inverters['ABB__MICRO_0_25_I_OUTD_US_208_208V__CEC_2014_'].copy()
4649
system = PVSystem(surface_tilt=32.2, surface_azimuth=180,
50+
module=module,
4751
module_parameters=module_parameters,
4852
inverter_parameters=inverter)
4953
return system
@@ -52,13 +56,15 @@ def cec_dc_snl_ac_system(sam_data):
5256
@pytest.fixture
5357
def cec_dc_adr_ac_system(sam_data):
5458
modules = sam_data['cecmod']
55-
module_parameters = modules['Canadian_Solar_CS5P_220M'].copy()
59+
module = 'Canadian_Solar_CS5P_220M'
60+
module_parameters = modules[module].copy()
5661
module_parameters['b'] = 0.05
5762
module_parameters['EgRef'] = 1.121
5863
module_parameters['dEgdT'] = -0.0002677
5964
inverters = sam_data['adrinverter']
6065
inverter = inverters['Zigor__Sunzet_3_TL_US_240V__CEC_2011_'].copy()
6166
system = PVSystem(surface_tilt=32.2, surface_azimuth=180,
67+
module=module,
6268
module_parameters=module_parameters,
6369
inverter_parameters=inverter)
6470
return system
@@ -365,6 +371,27 @@ def test_losses_models_no_loss(pvwatts_dc_pvwatts_ac_system, location, weather,
365371
assert mc.losses == 1
366372

367373

374+
def test_invalid_dc_model_params(system, cec_dc_snl_ac_system,
375+
pvwatts_dc_pvwatts_ac_system, location):
376+
kwargs = {'dc_model': 'sapm', 'ac_model': 'snlinverter',
377+
'aoi_model': 'no_loss', 'spectral_model': 'no_loss',
378+
'temp_model': 'sapm', 'losses_model': 'no_loss'}
379+
system.module_parameters.pop('A0') # remove a parameter
380+
with pytest.raises(ValueError):
381+
mc = ModelChain(system, location, **kwargs)
382+
383+
kwargs['dc_model'] = 'singlediode'
384+
cec_dc_snl_ac_system.module_parameters.pop('a_ref') # remove a parameter
385+
with pytest.raises(ValueError):
386+
mc = ModelChain(cec_dc_snl_ac_system, location, **kwargs)
387+
388+
kwargs['dc_model'] = 'pvwatts'
389+
kwargs['ac_model'] = 'pvwatts'
390+
pvwatts_dc_pvwatts_ac_system.module_parameters.pop('pdc0')
391+
with pytest.raises(ValueError):
392+
mc = ModelChain(pvwatts_dc_pvwatts_ac_system, location, **kwargs)
393+
394+
368395
@pytest.mark.parametrize('model', [
369396
'dc_model', 'ac_model', 'aoi_model', 'spectral_model', 'losses_model',
370397
'temp_model', 'losses_model'

0 commit comments

Comments
 (0)