refactor ModelChain inverter methods to use PVSystem.get_ac (#1150)

* deprecate methods

* remove new but unreleased methods

* refactor modelchain inverter methods for get_ac

* update whats new

* update api.rst

* remove tests for methods that no longer exist

* fix get_ac with sandia tests

* fix get_ac with sandia tests

* clean up pvsystem imports

* add test of infer_ac_model

* no v in deprecated args

* fix pull number....

* remove _infer_ac_model_multi

Author: wholmgren

docs/sphinx/source/api.rst

@@ -630,8 +630,8 @@ ModelChain model definitions.
    modelchain.ModelChain.desoto
    modelchain.ModelChain.pvsyst
    modelchain.ModelChain.pvwatts_dc
-   modelchain.ModelChain.snlinverter
-   modelchain.ModelChain.adrinverter
+   modelchain.ModelChain.sandia_inverter
+   modelchain.ModelChain.adr_inverter
    modelchain.ModelChain.pvwatts_inverter
    modelchain.ModelChain.ashrae_aoi_loss
    modelchain.ModelChain.physical_aoi_loss

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

@@ -36,6 +36,10 @@ Breaking changes
 * ``irradiance.liujordan`` and ``ForecastModel.cloud_cover_to_irradiance_liujordan``
   have been removed. (:pull:`1136`)
 
+* ``ModelChain.snlinverter`` changed to ``ModelChain.sandia_inverter``.
+  ``ModelChain.adrinverter`` changed to ``ModelChain.adr_inverter``.
+  (:pull:`1150`)
+
 
 Deprecations
 ~~~~~~~~~~~~
@@ -76,9 +80,9 @@ Enhancements
 * Support for :py:func:`~pvlib.inverter.sandia_multi` and
   :py:func:`~pvlib.inverter.pvwatts_multi` added to
   :py:class:`~pvlib.pvsystem.PVSystem` and
-  :py:class:`~pvlib.modelchain.ModelChain` (as ``ac_model='sandia_multi'``
-  and  ``ac_model='pvwatts_multi'``).
-  (:pull:`1076`, :issue:`1067`, :pull:`1132`, :issue:`1117`)
+  :py:class:`~pvlib.modelchain.ModelChain` (as ``ac_model='sandia'``
+  and ``ac_model='pvwatts'``).
+  (:pull:`1076`, :issue:`1067`, :pull:`1132`, :issue:`1117`, :pull:`1150`)
 * :py:class:`~pvlib.modelchain.ModelChain` 'run_model' methods now
   automatically switch to using ``'effective_irradiance'`` (if available) for
   cell temperature models, when ``'poa_global'`` is not provided in input
@@ -88,8 +92,8 @@ Enhancements
   ``pvsystem.PVSystem.strings_per_inverter``. Note that both attributes still
   default to 1. (:pull:`1138`)
 * :py:meth:`~pvlib.pvsystem.PVSystem.get_ac` is added to calculate AC power
-  from DC power. Use parameter 'model' to specify which inverter model to use.
-  (:pull:`1147`, :issue:`998`)
+  from DC power. Use parameter ``model`` to specify which inverter model to use.
+  (:pull:`1147`, :issue:`998`, :pull:`1150`)
 
 Bug fixes
 ~~~~~~~~~

pvlib/modelchain.py

@@ -770,15 +770,11 @@ def ac_model(self, model):
         elif isinstance(model, str):
             model = model.lower()
             if model == 'sandia':
-                self._ac_model = self.snlinverter
-            elif model == 'sandia_multi':
-                self._ac_model = self.sandia_multi_inverter
+                self._ac_model = self.sandia_inverter
             elif model in 'adr':
-                self._ac_model = self.adrinverter
+                self._ac_model = self.adr_inverter
             elif model == 'pvwatts':
                 self._ac_model = self.pvwatts_inverter
-            elif model == 'pvwatts_multi':
-                self._ac_model = self.pvwatts_multi_inverter
             else:
                 raise ValueError(model + ' is not a valid AC power model')
         else:
@@ -787,53 +783,41 @@ def ac_model(self, model):
     def infer_ac_model(self):
         """Infer AC power model from system attributes."""
         inverter_params = set(self.system.inverter_parameters.keys())
-        if self.system.num_arrays > 1:
-            return self._infer_ac_model_multi(inverter_params)
         if _snl_params(inverter_params):
-            return self.snlinverter
+            return self.sandia_inverter
         if _adr_params(inverter_params):
-            return self.adrinverter
+            if self.system.num_arrays > 1:
+                raise ValueError(
+                    'The adr inverter function cannot be used for an inverter',
+                    ' with multiple MPPT inputs')
+            else:
+                return self.adr_inverter
         if _pvwatts_params(inverter_params):
             return self.pvwatts_inverter
         raise ValueError('could not infer AC model from '
                          'system.inverter_parameters. Check '
                          'system.inverter_parameters or explicitly '
                          'set the model with the ac_model kwarg.')
 
-    def _infer_ac_model_multi(self, inverter_params):
-        if _snl_params(inverter_params):
-            return self.sandia_multi_inverter
-        elif _pvwatts_params(inverter_params):
-            return self.pvwatts_multi_inverter
-        raise ValueError('could not infer multi-array AC model from '
-                         'system.inverter_parameters. Only sandia and pvwatts '
-                         'inverter models support multiple '
-                         'Arrays. Check system.inverter_parameters or '
-                         'explicitly set the model with the ac_model kwarg.')
-
-    def sandia_multi_inverter(self):
-        self.results.ac = self.system.sandia_multi(
-            _tuple_from_dfs(self.results.dc, 'v_mp'),
-            _tuple_from_dfs(self.results.dc, 'p_mp')
+    def sandia_inverter(self):
+        self.results.ac = self.system.get_ac(
+            'sandia',
+            _tuple_from_dfs(self.results.dc, 'p_mp'),
+            v_dc=_tuple_from_dfs(self.results.dc, 'v_mp')
         )
         return self
 
-    def pvwatts_multi_inverter(self):
-        self.results.ac = self.system.pvwatts_multi(self.results.dc)
-        return self
-
-    def snlinverter(self):
-        self.results.ac = self.system.snlinverter(self.results.dc['v_mp'],
-                                                  self.results.dc['p_mp'])
-        return self
-
-    def adrinverter(self):
-        self.results.ac = self.system.adrinverter(self.results.dc['v_mp'],
-                                                  self.results.dc['p_mp'])
+    def adr_inverter(self):
+        self.results.ac = self.system.get_ac(
+            'adr',
+            self.results.dc['p_mp'],
+            v_dc=self.results.dc['v_mp']
+        )
         return self
 
     def pvwatts_inverter(self):
-        self.results.ac = self.system.pvwatts_ac(self.results.dc).fillna(0)
+        ac = self.system.get_ac('pvwatts', self.results.dc)
+        self.results.ac = ac.fillna(0)
         return self
 
     @property

pvlib/pvsystem.py

@@ -8,7 +8,6 @@
 import io
 import os
 from urllib.request import urlopen
-import warnings
 import numpy as np
 import pandas as pd
 
@@ -17,7 +16,6 @@
 from pvlib import (atmosphere, iam, inverter, irradiance,
                    singlediode as _singlediode, temperature)
 from pvlib.tools import _build_kwargs
-from pvlib._deprecation import pvlibDeprecationWarning
 
 
 # a dict of required parameter names for each DC power model
@@ -921,6 +919,7 @@ def get_ac(self, model, p_dc, v_dc=None):
                 model + ' is not a valid AC power model.',
                 ' model must be one of "sandia", "adr" or "pvwatts"')
 
+    @deprecated('0.9', alternative='PVSystem.get_ac', removal='0.10')
     def snlinverter(self, v_dc, p_dc):
         """Uses :py:func:`pvlib.inverter.sandia` to calculate AC power based on
         ``self.inverter_parameters`` and the input voltage and power.
@@ -929,19 +928,7 @@ def snlinverter(self, v_dc, p_dc):
         """
         return inverter.sandia(v_dc, p_dc, self.inverter_parameters)
 
-    def sandia_multi(self, v_dc, p_dc):
-        """Uses :py:func:`pvlib.inverter.sandia_multi` to calculate AC power
-        based on ``self.inverter_parameters`` and the input voltage and power.
-
-        The parameters `v_dc` and `p_dc` must be tuples with length equal to
-        ``self.num_arrays`` if the system has more than one array.
-
-        See :py:func:`pvlib.inverter.sandia_multi` for details.
-        """
-        v_dc = self._validate_per_array(v_dc)
-        p_dc = self._validate_per_array(p_dc)
-        return inverter.sandia_multi(v_dc, p_dc, self.inverter_parameters)
-
+    @deprecated('0.9', alternative='PVSystem.get_ac', removal='0.10')
     def adrinverter(self, v_dc, p_dc):
         """Uses :py:func:`pvlib.inverter.adr` to calculate AC power based on
         ``self.inverter_parameters`` and the input voltage and power.
@@ -1009,6 +996,7 @@ def pvwatts_losses(self):
                                self.losses_parameters)
         return pvwatts_losses(**kwargs)
 
+    @deprecated('0.9', alternative='PVSystem.get_ac', removal='0.10')
     def pvwatts_ac(self, pdc):
         """
         Calculates AC power according to the PVWatts model using
@@ -1023,20 +1011,6 @@ def pvwatts_ac(self, pdc):
         return inverter.pvwatts(pdc, self.inverter_parameters['pdc0'],
                                 **kwargs)
 
-    def pvwatts_multi(self, p_dc):
-        """Uses :py:func:`pvlib.inverter.pvwatts_multi` to calculate AC power
-        based on ``self.inverter_parameters`` and the input voltage and power.
-
-        The parameter `p_dc` must be a tuple with length equal to
-        ``self.num_arrays`` if the system has more than one array.
-
-        See :py:func:`pvlib.inverter.pvwatts_multi` for details.
-        """
-        p_dc = self._validate_per_array(p_dc)
-        kwargs = _build_kwargs(['eta_inv_nom', 'eta_inv_ref'],
-                               self.inverter_parameters)
-        return inverter.pvwatts_multi(p_dc, self.inverter_parameters['pdc0'],
-                                      **kwargs)
     @property
     @_unwrap_single_value
     def module_parameters(self):

pvlib/tests/test_modelchain.py

@@ -3,7 +3,7 @@
 import numpy as np
 import pandas as pd
 
-from pvlib import iam, modelchain, pvsystem, temperature
+from pvlib import iam, modelchain, pvsystem, temperature, inverter
 from pvlib.modelchain import ModelChain
 from pvlib.pvsystem import PVSystem
 from pvlib.tracking import SingleAxisTracker
@@ -480,7 +480,7 @@ def test_ModelChain_invalid_inverter_params_arrays(
     sapm_dc_snl_ac_system_same_arrays.inverter_parameters = \
         inverter_params[inverter]
     with pytest.raises(ValueError,
-                       match=r'Only sandia and pvwatts inverter models'):
+                       match=r'adr inverter function cannot'):
         ModelChain(sapm_dc_snl_ac_system_same_arrays, location)
 
 
@@ -1235,27 +1235,36 @@ def acdc(mc):
     mc.results.ac = mc.results.dc
 
 
-@pytest.mark.parametrize('ac_model', ['sandia', 'adr',
-                                      'pvwatts', 'sandia_multi',
-                                      'pvwatts_multi'])
+@pytest.mark.parametrize('inverter_model', ['sandia', 'adr',
+                                            'pvwatts', 'sandia_multi',
+                                            'pvwatts_multi'])
 def test_ac_models(sapm_dc_snl_ac_system, cec_dc_adr_ac_system,
-                   pvwatts_dc_pvwatts_ac_system, location, ac_model,
-                   weather, mocker):
+                   pvwatts_dc_pvwatts_ac_system, cec_dc_snl_ac_arrays,
+                   pvwatts_dc_pvwatts_ac_system_arrays,
+                   location, inverter_model, weather, mocker):
     ac_systems = {'sandia': sapm_dc_snl_ac_system,
-                  'sandia_multi': sapm_dc_snl_ac_system,
+                  'sandia_multi': cec_dc_snl_ac_arrays,
                   'adr': cec_dc_adr_ac_system,
                   'pvwatts': pvwatts_dc_pvwatts_ac_system,
-                  'pvwatts_multi': pvwatts_dc_pvwatts_ac_system}
-    ac_method_name = {'sandia': 'snlinverter',
-                      'sandia_multi': 'sandia_multi',
-                      'adr': 'adrinverter',
-                      'pvwatts': 'pvwatts_ac',
-                      'pvwatts_multi': 'pvwatts_multi'}
-    system = ac_systems[ac_model]
-
+                  'pvwatts_multi': pvwatts_dc_pvwatts_ac_system_arrays}
+    inverter_to_ac_model = {
+        'sandia': 'sandia',
+        'sandia_multi': 'sandia',
+        'adr': 'adr',
+        'pvwatts': 'pvwatts',
+        'pvwatts_multi': 'pvwatts'}
+    ac_model = inverter_to_ac_model[inverter_model]
+    system = ac_systems[inverter_model]
+
+    mc_inferred = ModelChain(system, location,
+                             aoi_model='no_loss', spectral_model='no_loss')
     mc = ModelChain(system, location, ac_model=ac_model,
                     aoi_model='no_loss', spectral_model='no_loss')
-    m = mocker.spy(system, ac_method_name[ac_model])
+
+    # tests ModelChain.infer_ac_model
+    assert mc_inferred.ac_model.__name__ == mc.ac_model.__name__
+
+    m = mocker.spy(inverter, inverter_model)
     mc.run_model(weather)
     assert m.call_count == 1
     assert isinstance(mc.results.ac, pd.Series)
@@ -1447,7 +1456,7 @@ def test_losses_models_no_loss(pvwatts_dc_pvwatts_ac_system, location, weather,
 
 def test_invalid_dc_model_params(sapm_dc_snl_ac_system, cec_dc_snl_ac_system,
                                  pvwatts_dc_pvwatts_ac_system, location):
-    kwargs = {'dc_model': 'sapm', 'ac_model': 'snlinverter',
+    kwargs = {'dc_model': 'sapm', 'ac_model': 'sandia',
               'aoi_model': 'no_loss', 'spectral_model': 'no_loss',
               'temperature_model': 'sapm', 'losses_model': 'no_loss'}
     sapm_dc_snl_ac_system.module_parameters.pop('A0')  # remove a parameter
@@ -1488,9 +1497,9 @@ def test_bad_get_orientation():
 def test_with_sapm_pvsystem_arrays(sapm_dc_snl_ac_system_Array, location,
                                    weather):
     mc = ModelChain.with_sapm(sapm_dc_snl_ac_system_Array, location,
-                              ac_model='sandia_multi')
+                              ac_model='sandia')
     assert mc.dc_model == mc.sapm
-    assert mc.ac_model == mc.sandia_multi_inverter
+    assert mc.ac_model == mc.sandia_inverter
     mc.run_model(weather)
     assert mc.results
 
@@ -1625,7 +1634,7 @@ def test_ModelChain___repr__(sapm_dc_snl_ac_system, location, strategy,
         '  solar_position_method: nrel_numpy',
         '  airmass_model: kastenyoung1989',
         '  dc_model: sapm',
-        '  ac_model: snlinverter',
+        '  ac_model: sandia_inverter',
         '  aoi_model: sapm_aoi_loss',
         '  spectral_model: sapm_spectral_loss',
         '  temperature_model: sapm_temp',
@@ -1778,7 +1787,7 @@ def test_inconsistent_array_params(location,
     )
     with pytest.raises(ValueError, match=temperature_error):
         ModelChain(different_temp_system, location,
-                   ac_model='sandia_multi',
+                   ac_model='sandia',
                    aoi_model='no_loss', spectral_model='no_loss',
                    temperature_model='sapm')