[BUG]: don't infer spectral model in ModelChain (#2253)

* Fix #2017

* rm TODOs

* cwhanse review

Co-Authored-By: Cliff Hansen <[email protected]>

* typo

* pep8 compliance

* ups

* Update modelchain.py

* Infer a string, key for setter

* pep8 linting

* Remove delayed-inference code - codecov warning

* default to no_loss

* infer_spectral_model tweak -> return list if possible

* remove infer_spectral_model

* lint

* review

* remove reference to infer_spectral_model method

---------

Co-authored-by: Cliff Hansen <[email protected]>
Co-authored-by: Cliff Hansen <[email protected]>

Author: echedey-ls

docs/sphinx/source/reference/modelchain.rst

@@ -112,7 +112,6 @@ on the information in the associated :py:class:`~pvsystem.PVSystem` object.
    modelchain.ModelChain.infer_dc_model
    modelchain.ModelChain.infer_ac_model
    modelchain.ModelChain.infer_aoi_model
-   modelchain.ModelChain.infer_spectral_model
    modelchain.ModelChain.infer_temperature_model
    modelchain.ModelChain.infer_losses_model
 

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

@@ -4,6 +4,15 @@
 v0.11.3 (Anticipated March, 2025)
 ---------------------------------
 
+Breaking Changes
+~~~~~~~~~~~~~~~~
+* The pvlib.location.Location.pytz attribute is now read only. The
+  pytz attribute is now set internally to be consistent with the
+  pvlib.location.Location.tz attribute. (:issue:`2340`, :pull:`2341`)
+* Users must now provide ModelChain.spectral_model, or the 'no_loss' spectral
+  model is assumed. pvlib.modelchain.ModelChain no longer attempts to infer
+  the spectral model from PVSystem attributes. (:issue:`2017`, :pull:`2253`)
+
 Bug fixes
 ~~~~~~~~~
 * Fix a bug in :py:func:`pvlib.bifacial.get_irradiance_poa` which may have yielded non-zero
@@ -62,11 +71,6 @@ Maintenance
 * asv 0.4.2 upgraded to asv 0.6.4 to fix CI failure due to pinned older conda.
   (:pull:`2352`)
 
-Breaking Changes
-~~~~~~~~~~~~~~~~
-* The pvlib.location.Location.pytz attribute is now read only. The
-  pytz attribute is now set internally to be consistent with the
-  pvlib.location.Location.tz attribute. (:issue:`2340`, :pull:`2341`)
 
 Contributors
 ~~~~~~~~~~~~

pvlib/modelchain.py

@@ -29,7 +29,7 @@
 
 # Optional keys to communicate temperature data. If provided,
 # 'cell_temperature' overrides ModelChain.temperature_model and sets
-# ModelChain.cell_temperature to the data. If 'module_temperature' is provdied,
+# ModelChain.cell_temperature to the data. If 'module_temperature' is provided,
 # overrides ModelChain.temperature_model with
 # pvlib.temperature.sapm_celL_from_module
 TEMPERATURE_KEYS = ('module_temperature', 'cell_temperature')
@@ -253,7 +253,7 @@ def __repr__(self):
         def _head(obj):
             try:
                 return obj[:3]
-            except:
+            except Exception:
                 return obj
 
         if type(self.dc) is tuple:
@@ -269,7 +269,7 @@ def _head(obj):
                  '\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,12 +330,15 @@ class ModelChain:
         '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.
+        a user-defined function. If not specified, ``'no_loss'`` is assumed.
 
     temperature_model : str or function, optional
         Valid strings are: 'sapm', 'pvsyst', 'faiman', 'fuentes', 'noct_sam'.
@@ -386,7 +389,6 @@ def __init__(self, system, location,
 
         self.results = ModelChainResult()
 
-
     @classmethod
     def with_pvwatts(cls, system, location,
                      clearsky_model='ineichen',
@@ -855,9 +857,7 @@ def spectral_model(self):
 
     @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,30 +867,12 @@ def spectral_model(self, model):
                 self._spectral_model = self.no_spectral_loss
             else:
                 raise ValueError(model + ' is not a valid spectral loss model')
-        else:
+        elif model is None:
+            # not setting a model is equivalent to setting no_loss
+            self._spectral_model = self.no_spectral_loss
+        else:  # assume model is callable with 1st argument = the MC instance
             self._spectral_model = partial(model, self)
 
-    def infer_spectral_model(self):
-        """Infer spectral model from system attributes."""
-        module_parameters = tuple(
-            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".')
-
     def first_solar_spectral_loss(self):
         self.results.spectral_modifier = self.system.first_solar_spectral_loss(
             _tuple_from_dfs(self.results.weather, 'precipitable_water'),
@@ -1570,7 +1552,7 @@ def _prepare_temperature(self, data):
         ----------
         data : DataFrame
             May contain columns ``'cell_temperature'`` or
-            ``'module_temperaure'``.
+            ``'module_temperature'``.
 
         Returns
         -------
@@ -1679,6 +1661,7 @@ def run_model(self, weather):
         self.prepare_inputs(weather)
         self.aoi_model()
         self.spectral_model()
+
         self.effective_irradiance_model()
 
         self._run_from_effective_irrad(weather)

tests/test_modelchain.py

@@ -346,7 +346,7 @@ def test_with_pvwatts(pvwatts_dc_pvwatts_ac_system, location, weather):
 
 
 def test_run_model_with_irradiance(sapm_dc_snl_ac_system, location):
-    mc = ModelChain(sapm_dc_snl_ac_system, location)
+    mc = ModelChain(sapm_dc_snl_ac_system, location, spectral_model='sapm')
     times = pd.date_range('20160101 1200-0700', periods=2, freq='6h')
     irradiance = pd.DataFrame({'dni': 900, 'ghi': 600, 'dhi': 150},
                               index=times)
@@ -629,9 +629,13 @@ def test_run_model_arrays_weather(sapm_dc_snl_ac_system_same_arrays,
 
 
 def test_run_model_perez(sapm_dc_snl_ac_system, location):
-    mc = ModelChain(sapm_dc_snl_ac_system, location,
-                    transposition_model='perez')
-    times = pd.date_range('20160101 1200-0700', periods=2, freq='6h')
+    mc = ModelChain(
+        sapm_dc_snl_ac_system,
+        location,
+        transposition_model="perez",
+        spectral_model="sapm",
+    )
+    times = pd.date_range("20160101 1200-0700", periods=2, freq="6h")
     irradiance = pd.DataFrame({'dni': 900, 'ghi': 600, 'dhi': 150},
                               index=times)
     ac = mc.run_model(irradiance).results.ac
@@ -642,10 +646,14 @@ def test_run_model_perez(sapm_dc_snl_ac_system, location):
 
 
 def test_run_model_gueymard_perez(sapm_dc_snl_ac_system, location):
-    mc = ModelChain(sapm_dc_snl_ac_system, location,
-                    airmass_model='gueymard1993',
-                    transposition_model='perez')
-    times = pd.date_range('20160101 1200-0700', periods=2, freq='6h')
+    mc = ModelChain(
+        sapm_dc_snl_ac_system,
+        location,
+        airmass_model="gueymard1993",
+        transposition_model="perez",
+        spectral_model="sapm",
+    )
+    times = pd.date_range("20160101 1200-0700", periods=2, freq="6h")
     irradiance = pd.DataFrame({'dni': 900, 'ghi': 600, 'dhi': 150},
                               index=times)
     ac = mc.run_model(irradiance).results.ac
@@ -1272,18 +1280,6 @@ def test_singlediode_dc_arrays(location, dc_model,
         assert isinstance(dc, (pd.Series, pd.DataFrame))
 
 
-@pytest.mark.parametrize('dc_model', ['sapm', 'cec', 'cec_native'])
-def test_infer_spectral_model(location, sapm_dc_snl_ac_system,
-                              cec_dc_snl_ac_system,
-                              cec_dc_native_snl_ac_system, dc_model):
-    dc_systems = {'sapm': sapm_dc_snl_ac_system,
-                  'cec': cec_dc_snl_ac_system,
-                  'cec_native': cec_dc_native_snl_ac_system}
-    system = dc_systems[dc_model]
-    mc = ModelChain(system, location, aoi_model='physical')
-    assert isinstance(mc, ModelChain)
-
-
 @pytest.mark.parametrize('temp_model', [
     'sapm_temp', 'faiman_temp', 'pvsyst_temp', 'fuentes_temp',
     'noct_sam_temp'])
@@ -2002,9 +1998,9 @@ def test__irrad_for_celltemp():
 
 
 def test_ModelChain___repr__(sapm_dc_snl_ac_system, location):
-
-    mc = ModelChain(sapm_dc_snl_ac_system, location,
-                    name='my mc')
+    mc = ModelChain(
+        sapm_dc_snl_ac_system, location, name="my mc", spectral_model="sapm"
+    )
 
     expected = '\n'.join([
         'ModelChain: ',