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

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

@@ -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
@@ -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

pvlib/modelchain.py

@@ -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,
@@ -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:
@@ -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')

pvlib/pvsystem.py

@@ -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.

pvlib/test/test_modelchain.py

@@ -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
@@ -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
@@ -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
@@ -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'