8000 add consistency tests to ModelChain.dc_model by cwhanse · Pull Request #548 · pvlib/pvlib-python · GitHub
[go: up one dir, main page]
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.

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 13 commits into from
Aug 31, 2018
Merged

add consistency tests to ModelChain.dc_model #548

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d3c1ae3
add consistency tests to ModelChain.dc_model
cwhanse Aug 27, 2018
6884555
Add self argument
cwhanse Aug 27, 2018
1c4ac04
Move DC_MODEL_PARAMS to pvsystem, streamline validation
cwhanse Aug 28, 2018
a04e111
Fix key for DC_MODEL_PARAMS
cwhanse Aug 28, 2018
3e1116b
Fix typo in IXO, IXXO
cwhanse Aug 28, 2018
1fa1be3
Change logic in dc_model, correct pvwatts keyword
cwhanse Aug 28, 2018
121a8b1
More adjustment to pvwatts_dc vs pvwatts keyword
cwhanse Aug 28, 2018
10a5f7d
Add handling of invalid model name string
cwhanse Aug 29, 2018
c0f58c3
Add test for invalid dc model parameters
cwhanse Aug 30, 2018
ae3d572
Fix typo in cec_dc_adr_ac_system
cwhanse Aug 30, 2018
e11d1c5
Add to whatsnew.rst
cwhanse Aug 30, 2018
743c621
Cleanup, remove copy from test
cwhanse Aug 30, 2018
669d4bd
Delete comment from infer_dc_model
cwhanse Aug 30, 2018
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
1 change: 1 addition & 0 deletions docs/sphinx/source/whatsnew/v0.6.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,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
38 changes: 27 additions & 11 deletions pvlib/modelchain.py
View file
Open in desktop
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,29 +361,44 @@ 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 : ' +
Copy link
Member

Choose a reason for hiding this comment

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

space in string at end of previous line and beginning of this one, so remove one of them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

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:
self._dc_model = partial(model, self)

def infer_dc_model(self):
# returns both model function object and model string, could drop
# model function object in the future since the model function object
# will be set in dc_model after validating parameter consistency
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
View file
Open in desktop
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
40 changes: 36 additions & 4 deletions pvlib/test/test_modelchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,18 @@
from test_pvsystem import sam_data
from conftest import requires_scipy

import copy


@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 +41,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_paramet 628C ers=module_parameters,
inverter_parameters=inverter)
return system
Expand All @@ -52,13 +58,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 +373,30 @@ 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'}
tmp = copy.deepcopy(system)
Copy link
Member

Choose a reason for hiding this comment

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

Did you find that it was necessary to use copy here for the test to pass with pytest? I am surprised because the test fixtures should generate a new system with fresh data for each test. You're not reusing them within this test so I don't see why it would be necessary. I can see that if you were developing this test function in the interpreter/notebook then the story would be different.

Copy link
Member Author
@cwhanse cwhanse Aug 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I didn't know that test fixtures are generated anew for each test. I used copy out of concern that pop-ing a key off the module_parameters dict would alter system etc. for other tests.

Removed.

tmp.module_parameters.pop('A0') # remove a parameter
with pytest.raises(ValueError):
mc = ModelChain(tmp, location, **kwargs)

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

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


@pytest.mark.parametrize('model', [
'dc_model', 'ac_model', 'aoi_model', 'spectral_model', 'losses_model',
'temp_model', 'losses_model'
Expand All @@ -376,7 +408,7 @@ def test_invalid_models(model, system, location):
kwargs[model] = 'invalid'
with pytest.raises(ValueError):
mc = ModelChain(system, location, **kwargs)

Copy link
Member

Choose a reason for hiding this comment

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

See if you can set your editor to remove extra white space from lines when saving. It makes git diffs unnecessarily busy.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


def test_bad_get_orientation():
with pytest.raises(ValueError):
Expand Down
0