[BUG]: Infer spectral models correctly

pvlib/modelchain.py

@@ -253,7 +253,7 @@
         def _head(obj):
             try:
                 return obj[:3]
-            except:
+            except Exception:
                 return obj
 
         if type(self.dc) is tuple:
@@ -269,7 +269,7 @@
                  '\n')
         lines = []
         for attr in mc_attrs:
-            if not (attr.startswith('_') or attr=='times'):
+            if not (attr.startswith('_') or attr == 'times'):
                 lines.append(f' {attr}: ' + _mcr_repr(getattr(self, attr)))
         desc4 = '\n'.join(lines)
         return (desc1 + desc2 + desc3 + desc4)
@@ -330,13 +330,21 @@
         'interp' and 'no_loss'. The ModelChain instance will be passed as the
         first argument to a user-defined function.
 
-    spectral_model : str, or function, optional
-        If not specified, the model will be inferred from the parameters that
-        are common to all of system.arrays[i].module_parameters.
-        Valid strings are 'sapm', 'first_solar', 'no_loss'.
+    spectral_model : str or function, optional
+        Valid strings are:
+
+        - ``'sapm'``
+        - ``'first_solar'``
+        - ``'no_loss'``
+
         The ModelChain instance will be passed as the first argument to
         a user-defined function.
 
+        By default, it will be inferred from the system attributes only.
+
+        See :py:func:`~pvlib.modelchain.ModelChain.infer_spectral_model` to
+        infer the spectral model from system and weather information.
+
     temperature_model : str or function, optional
         Valid strings are: 'sapm', 'pvsyst', 'faiman', 'fuentes', 'noct_sam'.
         The ModelChain instance will be passed as the first argument to a
@@ -386,7 +394,6 @@
 
         self.results = ModelChainResult()
 
-
     @classmethod
     def with_pvwatts(cls, system, location,
                      clearsky_model='ineichen',
@@ -855,9 +862,7 @@
 
     @spectral_model.setter
     def spectral_model(self, model):
-        if model is None:
-            self._spectral_model = self.infer_spectral_model()
-        elif isinstance(model, str):
+        if isinstance(model, str):
             model = model.lower()
             if model == 'first_solar':
                 self._spectral_model = self.first_solar_spectral_loss
@@ -867,29 +872,55 @@
                 self._spectral_model = self.no_spectral_loss
             else:
                 raise ValueError(model + ' is not a valid spectral loss model')
-        else:
+        elif model is None:
+            # uses recursive setter to infer model, which returns a string
+            self.spectral_model = self.infer_spectral_model(weather=None)
+        else:  # assume model is a callable
             self._spectral_model = partial(model, self)
 
-    def infer_spectral_model(self):
-        """Infer spectral model from system attributes."""
+    def infer_spectral_model(self, weather=None):
+        """
+        Infer spectral model from system attributes, and optionally from
+        the input weather dataframe, and set it to the ``ModelChain`` instance.
+
+        Parameters
+        ----------
+        weather : pd.DataFrame or collection of str, optional
+            An object with columns of available input data to help infer
+            the spectral model. If ``None``, the spectral model will be
+            inferred from the system attributes only.
+
+        Returns
+        -------
+        Inferred spectral correction model : string key for model setter
+
+        Examples
+        --------
+        >>> mc = ModelChain(system, location)
+        >>> mc.spectral_model = mc.infer_spectral_model(weather=weather)
+        """
         module_parameters = tuple(
-            array.module_parameters for array in self.system.arrays)
+            array.module_parameters for array in self.system.arrays
+        )
         params = _common_keys(module_parameters)
-        if {'A4', 'A3', 'A2', 'A1', 'A0'} <= params:
-            return self.sapm_spectral_loss
-        elif ((('Technology' in params or
-                'Material' in params) and
-               (self.system._infer_cell_type() is not None)) or
-              'first_solar_spectral_coefficients' in params):
-            return self.first_solar_spectral_loss
-        else:
-            raise ValueError('could not infer spectral model from '
-                             'system.arrays[i].module_parameters. Check that '
-                             'the module_parameters for all Arrays in '
-                             'system.arrays contain valid '
-                             'first_solar_spectral_coefficients, a valid '
-                             'Material or Technology value, or set '
-                             'spectral_model="no_loss".')
+        if {"A4", "A3", "A2", "A1", "A0"} <= params:
+            return "sapm"
+        elif "first_solar_spectral_coefficients" in params:
+            # user explicitly sets spectral coefficients
+            return "first_solar"
+        elif (
+            # cell type is known or can be inferred
+            ("Technology" in params or "Material" in params)
+            and (self.system._infer_cell_type() is not None)
+        ):
+            # This suggests models that provide default parameters per cell
+            # type can be used. However, some models depend on other weather
+            # parameters, so we need to check if they are available.
+            if weather is not None:  # weather is available
+                if "precipitable_water" in weather:
+                    return "first_solar"
+
+        return "no_loss"
 
     def first_solar_spectral_loss(self):
         self.results.spectral_modifier = self.system.first_solar_spectral_loss(
@@ -1570,7 +1601,7 @@
         ----------
         data : DataFrame
             May contain columns ``'cell_temperature'`` or
-            ``'module_temperaure'``.
+            ``'module_temperature'``.
 
         Returns
         -------
@@ -1679,6 +1710,7 @@
         self.prepare_inputs(weather)
         self.aoi_model()
         self.spectral_model()
+
         self.effective_irradiance_model()
 
         self._run_from_effective_irrad(weather)

pvlib/tests/test_modelchain.py

@@ -7,13 +7,10 @@
 from pvlib.modelchain import ModelChain
 from pvlib.pvsystem import PVSystem
 from pvlib.location import Location
-from pvlib._deprecation import pvlibDeprecationWarning
 
 from .conftest import assert_series_equal, assert_frame_equal
 import pytest
 
-from .conftest import fail_on_pvlib_version
-
 
 @pytest.fixture(scope='function')
 def sapm_dc_snl_ac_system(sapm_module_params, cec_inverter_parameters,
@@ -1271,6 +1268,28 @@ def test_infer_spectral_model(location, sapm_dc_snl_ac_system,
     assert isinstance(mc, ModelChain)
 
 
+def test_infer_spectral_model_with_weather(location, sapm_dc_snl_ac_system,
+                                           cec_dc_snl_ac_system, weather):
+    # instantiate example ModelChain to get the default spectral model
+    # inferred without weather available by default
+    # - should resolve to sapm
+    mc = ModelChain(sapm_dc_snl_ac_system, location, aoi_model='physical')
+    assert mc.spectral_model == mc.sapm_spectral_loss
+    # - should resolve to no loss
+    mc = ModelChain(cec_dc_snl_ac_system, location, aoi_model='physical')
+    assert mc.spectral_model == mc.no_spectral_loss
+
+    # infer spectral model from weather
+    # - without precipitable water in it, should resolve to no loss
+    mc.spectral_model = mc.infer_spectral_model(weather=weather)
+    assert mc.spectral_model == mc.no_spectral_loss
+    # - with precipitable water in it, should resolve to first solar
+    weather['precipitable_water'] = 1.42
+    mc.spectral_model = mc.infer_spectral_model(weather=weather)
+    assert mc.spectral_model == mc.first_solar_spectral_loss
+    assert isinstance(mc, ModelChain)
+
+
 @pytest.mark.parametrize('temp_model', [
     'sapm_temp', 'faiman_temp', 'pvsyst_temp', 'fuentes_temp',
     'noct_sam_temp'])