Repository: pvlib/pvlib-python (python)

Issue:
pvwatts loss model implementation with user defined input values 
Hi, 

I am trying to use pvwatts loss model with ModelChain with user defined loss values instead of the default ones. 

I understand that for default values I just have to pass "pvwatts" string as the losses_model but I am not sure how can I link the loss model with user-specified values in the ModelChain function. I have tried all the possible ways that I can think of but still not sure how this will be implemented. 

Will appreciate if anyone can guide me to it!  

Thanks in advance! 

Regards

Pull request description:
add PVSystem.losses_parameters, add support for custom pvwatts losses parameters in ModelChain
 - [x] Closes #484
 - [x] Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests must pass on the TravisCI and Appveyor testing services.
 - [x] Code quality and style is sufficient. Passes ``git diff upstream/master -u -- "*.py" | flake8 --diff`` and/or landscape.io linting service.
 - [x] New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
 - [x] Updates entries to `docs/sphinx/source/api.rst` for API changes.
 - [x] Adds description and name entries in the appropriate `docs/sphinx/source/whatsnew` file for all changes.

Added tests to ensure that custom losses are now supported in ModelChain.

Patch:
diff --git a/docs/sphinx/source/whatsnew/v0.6.0.rst b/docs/sphinx/source/whatsnew/v0.6.0.rst
index 009f015..2c9038f 100644
--- a/docs/sphinx/source/whatsnew/v0.6.0.rst
+++ b/docs/sphinx/source/whatsnew/v0.6.0.rst
@@ -5,7 +5,11 @@ v0.6.0 (___, 2018)
 
 API Changes
 ~~~~~~~~~~~
-* pvsystem.calcparams_desoto now requires arguments for each module model parameter.
+* pvsystem.calcparams_desoto now requires arguments for each module model
+  parameter. (:issue:`462`)
+* Add losses_parameters attribute to PVSystem objects and remove the kwargs
+  support from PVSystem.pvwatts_losses. Enables custom losses specification
+  in ModelChain calculations. (:issue:`484`)
 * removed irradiance parameter from ModelChain.run_model and ModelChain.prepare_inputs
 
 
@@ -59,7 +63,8 @@ Bug fixes
   Location.get_clearsky. Fixed. (:issue:`481`)
 * Add User-Agent specification to TMY3 remote requests to avoid rejection.
   (:issue:`493`)
-
+* Fix ``pvlib.irradiance.klucher`` output is different for Pandas Series vs.
+  floats and NumPy arrays. (:issue:`508`)
 
 Documentation
 ~~~~~~~~~~~~~
diff --git a/pvlib/irradiance.py b/pvlib/irradiance.py
index fd03c3f..3006071 100644
--- a/pvlib/irradiance.py
+++ b/pvlib/irradiance.py
@@ -629,7 +629,7 @@ def klucher(surface_tilt, surface_azimuth, dhi, ghi, solar_zenith,
         # fails with single point input
         F.fillna(0, inplace=True)
     except AttributeError:
-        F = 0
+        F = np.where(np.isnan(F), 0, F)
 
     term1 = 0.5 * (1 + tools.cosd(surface_tilt))
     term2 = 1 + F * (tools.sind(0.5 * surface_tilt) ** 3)
diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py
index 43c07de..e44617f 100644
--- a/pvlib/pvsystem.py
+++ b/pvlib/pvsystem.py
@@ -100,6 +100,9 @@ class PVSystem(object):
     racking_model : None or string, default 'open_rack_cell_glassback'
         Used for cell and module temperature calculations.
 
+    losses_parameters : None, dict or Series, default None
+        Losses parameters as defined by PVWatts or other.
+
     name : None or string, default None
 
     **kwargs
@@ -120,8 +123,7 @@ class PVSystem(object):
                  modules_per_string=1, strings_per_inverter=1,
                  inverter=None, inverter_parameters=None,
                  racking_model='open_rack_cell_glassback',
-                 name=None,
-                 **kwargs):
+                 losses_parameters=None, name=None, **kwargs):
 
         self.name = name
 
@@ -151,6 +153,11 @@ class PVSystem(object):
         else:
             self.inverter_parameters = inverter_parameters
 
+        if losses_parameters is None:
+            self.losses_parameters = {}
+        else:
+            self.losses_parameters = losses_parameters
+
         self.racking_model = racking_model
 
     def __repr__(self):
@@ -627,15 +634,17 @@ class PVSystem(object):
                           self.module_parameters['gamma_pdc'],
                           **kwargs)
 
-    def pvwatts_losses(self, **kwargs):
+    def pvwatts_losses(self):
         """
         Calculates DC power losses according the PVwatts model using
-        :py:func:`pvwatts_losses`. No attributes are used in this
-        calculation, but all keyword arguments will be passed to the
-        function.
+        :py:func:`pvwatts_losses` and ``self.losses_parameters``.`
 
         See :py:func:`pvwatts_losses` for details.
         """
+        kwargs = _build_kwargs(['soiling', 'shading', 'snow', 'mismatch',
+                                'wiring', 'connections', 'lid',
+                                'nameplate_rating', 'age', 'availability'],
+                               self.losses_parameters)
         return pvwatts_losses(**kwargs)
 
     def pvwatts_ac(self, pdc):


Test patch:
diff --git a/pvlib/test/test_irradiance.py b/pvlib/test/test_irradiance.py
index ef9599e..31ebbcb 100644
--- a/pvlib/test/test_irradiance.py
+++ b/pvlib/test/test_irradiance.py
@@ -114,8 +114,20 @@ def test_isotropic_series():
 
 
 def test_klucher_series_float():
-    result = irradiance.klucher(40, 180, 100, 900, 20, 180)
-    assert_allclose(result, 88.3022221559)
+    # klucher inputs
+    surface_tilt, surface_azimuth = 40.0, 180.0
+    dhi, ghi = 100.0, 900.0
+    solar_zenith, solar_azimuth = 20.0, 180.0
+    # expect same result for floats and pd.Series
+    expected = irradiance.klucher(
+        surface_tilt, surface_azimuth,
+        pd.Series(dhi), pd.Series(ghi),
+        pd.Series(solar_zenith), pd.Series(solar_azimuth)
+    )  # 94.99429931664851
+    result = irradiance.klucher(
+        surface_tilt, surface_azimuth, dhi, ghi, solar_zenith, solar_azimuth
+    )
+    assert_allclose(result, expected[0])
 
 
 def test_klucher_series():
@@ -123,6 +135,12 @@ def test_klucher_series():
                        ephem_data['apparent_zenith'],
                        ephem_data['azimuth'])
     assert_allclose(result, [0, 37.446276, 109.209347, 56.965916], atol=1e-4)
+    # expect same result for np.array and pd.Series
+    expected = irradiance.klucher(
+        40, 180, irrad_data['dhi'].values, irrad_data['ghi'].values,
+        ephem_data['apparent_zenith'].values, ephem_data['azimuth'].values
+    )
+    assert_allclose(result, expected, atol=1e-4)
 
 
 def test_haydavies():
diff --git a/pvlib/test/test_modelchain.py b/pvlib/test/test_modelchain.py
index 225063d..9c908e3 100644
--- a/pvlib/test/test_modelchain.py
+++ b/pvlib/test/test_modelchain.py
@@ -1,3 +1,5 @@
+import sys
+
 import numpy as np
 import pandas as pd
 from numpy import nan
@@ -78,11 +80,19 @@ def pvwatts_dc_pvwatts_ac_system(sam_data):
     return system
 
 
-@pytest.fixture()
+@pytest.fixture
 def location():
     return Location(32.2, -111, altitude=700)
 
 
+@pytest.fixture
+def weather():
+    times = pd.date_range('20160101 1200-0700', periods=2, freq='6H')
+    weather = pd.DataFrame({'ghi': [500, 0], 'dni': [800, 0], 'dhi': [100, 0]},
+                           index=times)
+    return weather
+
+
 def test_ModelChain_creation(system, location):
     mc = ModelChain(system, location)
 
@@ -292,7 +302,7 @@ def test_spectral_models(system, location, spectral_model):
                            columns=['precipitable_water'])
     mc = ModelChain(system, location, dc_model='sapm',
                     aoi_model='no_loss', spectral_model=spectral_model)
-    spectral_modifier = mc.run_model(times=times, 
+    spectral_modifier = mc.run_model(times=times,
                                      weather=weather).spectral_modifier
     assert isinstance(spectral_modifier, (pd.Series, float, int))
 
@@ -302,22 +312,44 @@ def constant_losses(mc):
     mc.ac *= mc.losses
 
 
-@requires_scipy
-@pytest.mark.parametrize('losses_model, expected', [
-    ('pvwatts', [163.280464174, 0]),
-    ('no_loss', [190.028186986, 0]),
-    (constant_losses, [171.025368287, 0])
-])
-def test_losses_models(pvwatts_dc_pvwatts_ac_system, location, losses_model,
-                       expected):
+def test_losses_models_pvwatts(pvwatts_dc_pvwatts_ac_system, location, weather,
+                               mocker):
+    age = 1
+    pvwatts_dc_pvwatts_ac_system.losses_parameters = dict(age=age)
+    m = mocker.spy(pvsystem, 'pvwatts_losses')
     mc = ModelChain(pvwatts_dc_pvwatts_ac_system, location, dc_model='pvwatts',
                     aoi_model='no_loss', spectral_model='no_loss',
-                    losses_model=losses_model)
-    times = pd.date_range('20160101 1200-0700', periods=2, freq='6H')
-    ac = mc.run_model(times).ac
+                    losses_model='pvwatts')
+    mc.run_model(weather.index, weather=weather)
+    assert m.call_count == 1
+    m.assert_called_with(age=age)
+    assert isinstance(mc.ac, (pd.Series, pd.DataFrame))
+    assert not mc.ac.empty
 
-    expected = pd.Series(np.array(expected), index=times)
-    assert_series_equal(ac, expected, check_less_precise=2)
+
+def test_losses_models_ext_def(pvwatts_dc_pvwatts_ac_system, location, weather,
+                               mocker):
+    m = mocker.spy(sys.modules[__name__], 'constant_losses')
+    mc = ModelChain(pvwatts_dc_pvwatts_ac_system, location, dc_model='pvwatts',
+                    aoi_model='no_loss', spectral_model='no_loss',
+                    losses_model=constant_losses)
+    mc.run_model(weather.index, weather=weather)
+    assert m.call_count == 1
+    assert isinstance(mc.ac, (pd.Series, pd.DataFrame))
+    assert mc.losses == 0.9
+    assert not mc.ac.empty
+
+
+def test_losses_models_no_loss(pvwatts_dc_pvwatts_ac_system, location, weather,
+                               mocker):
+    m = mocker.spy(pvsystem, 'pvwatts_losses')
+    mc = ModelChain(pvwatts_dc_pvwatts_ac_system, location, dc_model='pvwatts',
+                    aoi_model='no_loss', spectral_model='no_loss',
+                    losses_model='no_loss')
+    assert mc.losses_model == mc.no_extra_losses
+    mc.run_model(weather.index, weather=weather)
+    assert m.call_count == 0
+    assert mc.losses == 1
 
 
 @pytest.mark.parametrize('model', [
diff --git a/pvlib/test/test_pvsystem.py b/pvlib/test/test_pvsystem.py
index 729a654..91b90dd 100644
--- a/pvlib/test/test_pvsystem.py
+++ b/pvlib/test/test_pvsystem.py
@@ -1358,9 +1358,10 @@ def test_PVSystem_pvwatts_dc_kwargs(mocker):
 def test_PVSystem_pvwatts_losses(mocker):
     mocker.spy(pvsystem, 'pvwatts_losses')
     system = make_pvwatts_system_defaults()
-    expected = 15
     age = 1
-    out = system.pvwatts_losses(age=age)
+    system.losses_parameters = dict(age=age)
+    expected = 15
+    out = system.pvwatts_losses()
     pvsystem.pvwatts_losses.assert_called_once_with(age=age)
     assert out < expected