From ce2fb60807bb3aab85344b4ed6729a2b8b7999fd Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 12 Aug 2023 19:17:40 +0200 Subject: [PATCH 01/11] Make interp method available for modelchain --- pvlib/iam.py | 2 +- pvlib/modelchain.py | 26 ++++++++++++++++++-------- pvlib/pvsystem.py | 9 +++------ pvlib/tests/test_modelchain.py | 19 ++++++++++++++++++- 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/pvlib/iam.py b/pvlib/iam.py index b02d20989b..d72866e1e9 100644 --- a/pvlib/iam.py +++ b/pvlib/iam.py @@ -20,7 +20,7 @@ 'physical': {'n', 'K', 'L'}, 'martin_ruiz': {'a_r'}, 'sapm': {'B0', 'B1', 'B2', 'B3', 'B4', 'B5'}, - 'interp': set() + 'interp': {'theta_ref', 'iam_ref'} } diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index 3d2404689f..2d1bd3f790 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -14,10 +14,9 @@ from typing import Union, Tuple, Optional, TypeVar from pvlib import (atmosphere, clearsky, inverter, pvsystem, solarposition, - temperature) + temperature, iam) import pvlib.irradiance # avoid name conflict with full import from pvlib.pvsystem import _DC_MODEL_PARAMS -from pvlib._deprecation import pvlibDeprecationWarning from pvlib.tools import _build_kwargs from pvlib._deprecation import deprecated @@ -279,7 +278,7 @@ def _mcr_repr(obj): # scalar, None, other? return repr(obj) - + # Type for fields that vary between arrays T = TypeVar('T') @@ -490,7 +489,7 @@ class ModelChain: If None, the model will be inferred from the parameters that are common to all of system.arrays[i].module_parameters. Valid strings are 'physical', 'ashrae', 'sapm', 'martin_ruiz', - 'no_loss'. The ModelChain instance will be passed as the + 'interp' and 'no_loss'. The ModelChain instance will be passed as the first argument to a user-defined function. spectral_model: None, str, or function, default None @@ -917,6 +916,8 @@ def aoi_model(self, model): self._aoi_model = self.sapm_aoi_loss elif model == 'martin_ruiz': self._aoi_model = self.martin_ruiz_aoi_loss + elif model == 'interp': + self._aoi_model = self.interp_aoi_loss elif model == 'no_loss': self._aoi_model = self.no_aoi_loss else: @@ -928,14 +929,16 @@ def infer_aoi_model(self): module_parameters = tuple( array.module_parameters for array in self.system.arrays) params = _common_keys(module_parameters) - if {'K', 'L', 'n'} <= params: + if iam._IAM_MODEL_PARAMS['physical'] <= params: return self.physical_aoi_loss - elif {'B5', 'B4', 'B3', 'B2', 'B1', 'B0'} <= params: + elif iam._IAM_MODEL_PARAMS['sapm'] <= params: return self.sapm_aoi_loss - elif {'b'} <= params: + elif iam._IAM_MODEL_PARAMS['ashrae'] <= params: return self.ashrae_aoi_loss - elif {'a_r'} <= params: + elif iam._IAM_MODEL_PARAMS['martin_ruiz'] <= params: return self.martin_ruiz_aoi_loss + elif iam._IAM_MODEL_PARAMS['interp'] <= params: + return self.interp_aoi_loss else: raise ValueError('could not infer AOI model from ' 'system.arrays[i].module_parameters. Check that ' @@ -972,6 +975,13 @@ def martin_ruiz_aoi_loss(self): ) return self + def interp_aoi_loss(self): + self.results.aoi_modifier = self.system.get_iam( + self.results.aoi, + iam_model='interp' + ) + return self + def no_aoi_loss(self): if self.system.num_arrays == 1: self.results.aoi_modifier = 1.0 diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index 4fb0dc5772..9569e6bbef 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -388,7 +388,7 @@ def get_iam(self, aoi, iam_model='physical'): aoi_model : string, default 'physical' The IAM model to be used. Valid strings are 'physical', 'ashrae', - 'martin_ruiz' and 'sapm'. + 'martin_ruiz', 'sapm' and 'interp'. Returns ------- iam : numeric or tuple of numeric @@ -1151,7 +1151,7 @@ def get_iam(self, aoi, iam_model='physical'): aoi_model : string, default 'physical' The IAM model to be used. Valid strings are 'physical', 'ashrae', - 'martin_ruiz' and 'sapm'. + 'martin_ruiz', 'sapm' and 'interp'. Returns ------- @@ -1164,16 +1164,13 @@ def get_iam(self, aoi, iam_model='physical'): if `iam_model` is not a valid model name. """ model = iam_model.lower() - if model in ['ashrae', 'physical', 'martin_ruiz']: + if model in ['ashrae', 'physical', 'martin_ruiz', 'interp']: param_names = iam._IAM_MODEL_PARAMS[model] kwargs = _build_kwargs(param_names, self.module_parameters) func = getattr(iam, model) return func(aoi, **kwargs) elif model == 'sapm': return iam.sapm(aoi, self.module_parameters) - elif model == 'interp': - raise ValueError(model + ' is not implemented as an IAM model ' - 'option for Array') else: raise ValueError(model + ' is not a valid IAM model') diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index 7f7c94cb0e..7ed0faad49 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -1455,6 +1455,23 @@ def test_aoi_model_no_loss(sapm_dc_snl_ac_system, location, weather): assert mc.results.ac[1] < 1 +def test_aoi_model_interp(sapm_dc_snl_ac_system, location, weather, mocker): + # similar to test_aoi_models but requires arguments to work, so we + # add 'interp' aoi losses model arguments to module + sapm_dc_snl_ac_system.arrays[0].module_parameters['iam_ref'] = (1., 0.85) + sapm_dc_snl_ac_system.arrays[0].module_parameters['theta_ref'] = (0., 80.) + mc = ModelChain(sapm_dc_snl_ac_system, location, + dc_model='sapm', aoi_model='interp', + spectral_model='no_loss') + m = mocker.spy(sapm_dc_snl_ac_system, 'get_iam') + mc.run_model(weather=weather) + assert m.call_count == 1 + assert isinstance(mc.results.ac, pd.Series) + assert not mc.results.ac.empty + assert mc.results.ac[0] > 150 and mc.results.ac[0] < 200 + assert mc.results.ac[1] < 1 + + def test_aoi_model_user_func(sapm_dc_snl_ac_system, location, weather, mocker): m = mocker.spy(sys.modules[__name__], 'constant_aoi_loss') mc = ModelChain(sapm_dc_snl_ac_system, location, dc_model='sapm', @@ -1468,7 +1485,7 @@ def test_aoi_model_user_func(sapm_dc_snl_ac_system, location, weather, mocker): @pytest.mark.parametrize('aoi_model', [ - 'sapm', 'ashrae', 'physical', 'martin_ruiz' + 'sapm', 'ashrae', 'physical', 'martin_ruiz', 'interp' ]) def test_infer_aoi_model(location, system_no_aoi, aoi_model): for k in iam._IAM_MODEL_PARAMS[aoi_model]: From f562c28a2bd01972027002f64e6a8a86242f5e51 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 12 Aug 2023 20:04:51 +0200 Subject: [PATCH 02/11] Update test_pvsystem.py --- pvlib/tests/test_pvsystem.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pvlib/tests/test_pvsystem.py b/pvlib/tests/test_pvsystem.py index 754c906c21..35cb4b5b2c 100644 --- a/pvlib/tests/test_pvsystem.py +++ b/pvlib/tests/test_pvsystem.py @@ -64,10 +64,13 @@ def test_PVSystem_get_iam_sapm(sapm_module_params, mocker): assert_allclose(out, 1.0, atol=0.01) -def test_PVSystem_get_iam_interp(sapm_module_params, mocker): - system = pvsystem.PVSystem(module_parameters=sapm_module_params) - with pytest.raises(ValueError): - system.get_iam(45, iam_model='interp') +def test_PVSystem_get_iam_interp(): + custom_module_params = {'iam_ref': (1., 0.8), 'theta_ref': (0., 80.)} + system = pvsystem.PVSystem(module_parameters=custom_module_params) + aoi = ((0., 40., 80.),) + expected = (1., 0.9, 0.8) + out = system.get_iam(aoi, iam_model='interp') + assert_allclose(out, expected) def test__normalize_sam_product_names(): From ac7af3c622cf088560a2d882024a0c55b559b4ea Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 12 Aug 2023 20:06:02 +0200 Subject: [PATCH 03/11] Update v0.10.2.rst --- docs/sphinx/source/whatsnew/v0.10.2.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/sphinx/source/whatsnew/v0.10.2.rst b/docs/sphinx/source/whatsnew/v0.10.2.rst index ee25c29103..98e143d4a7 100644 --- a/docs/sphinx/source/whatsnew/v0.10.2.rst +++ b/docs/sphinx/source/whatsnew/v0.10.2.rst @@ -17,7 +17,9 @@ Enhancements (:pull:`1800`) * Added option to infer threshold values for :py:func:`pvlib.clearsky.detect_clearsky` (:issue:`1808`, :pull:`1784`) - +* Added :py:func:`~pvlib.iam.interp` option as AOI losses model in + :py:class:`pvlib.modelchain.ModelChain` and + :py:class:`pvlib.pvsystem.PVSystem`. (:issue:`1742`, :pull:`1832`) Bug fixes ~~~~~~~~~ @@ -45,3 +47,4 @@ Contributors * Adam R. Jensen (:ghuser:`AdamRJensen`) * Abigail Jones (:ghuser:`ajonesr`) * Taos Transue (:ghuser:`reepoi`) +* Echedey Luis (:ghuser:`echedey-ls`) From fc996ea956d1389061722fa1c2b81eeff7b13c91 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Tue, 15 Aug 2023 10:36:44 +0200 Subject: [PATCH 04/11] Apply Kevin's suggestions Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> --- docs/sphinx/source/whatsnew/v0.10.2.rst | 1 + pvlib/tests/test_modelchain.py | 10 ++++++---- pvlib/tests/test_pvsystem.py | 8 +++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/sphinx/source/whatsnew/v0.10.2.rst b/docs/sphinx/source/whatsnew/v0.10.2.rst index 98e143d4a7..df0c7447ec 100644 --- a/docs/sphinx/source/whatsnew/v0.10.2.rst +++ b/docs/sphinx/source/whatsnew/v0.10.2.rst @@ -48,3 +48,4 @@ Contributors * Abigail Jones (:ghuser:`ajonesr`) * Taos Transue (:ghuser:`reepoi`) * Echedey Luis (:ghuser:`echedey-ls`) +* Todd Karin (:ghuser:`toddkarin`) diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index 7ed0faad49..d640394cc6 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -1458,14 +1458,16 @@ def test_aoi_model_no_loss(sapm_dc_snl_ac_system, location, weather): def test_aoi_model_interp(sapm_dc_snl_ac_system, location, weather, mocker): # similar to test_aoi_models but requires arguments to work, so we # add 'interp' aoi losses model arguments to module - sapm_dc_snl_ac_system.arrays[0].module_parameters['iam_ref'] = (1., 0.85) - sapm_dc_snl_ac_system.arrays[0].module_parameters['theta_ref'] = (0., 80.) + iam_ref = (1., 0.85) + theta_ref = (0., 80.) + sapm_dc_snl_ac_system.arrays[0].module_parameters['iam_ref'] = iam_ref + sapm_dc_snl_ac_system.arrays[0].module_parameters['theta_ref'] = theta_ref mc = ModelChain(sapm_dc_snl_ac_system, location, dc_model='sapm', aoi_model='interp', spectral_model='no_loss') - m = mocker.spy(sapm_dc_snl_ac_system, 'get_iam') + m = mocker.spy(iam, 'interp') mc.run_model(weather=weather) - assert m.call_count == 1 + assert m.call_args.kwargs == {'iam_ref': iam_ref, 'theta_ref': theta_ref} assert isinstance(mc.results.ac, pd.Series) assert not mc.results.ac.empty assert mc.results.ac[0] > 150 and mc.results.ac[0] < 200 diff --git a/pvlib/tests/test_pvsystem.py b/pvlib/tests/test_pvsystem.py index 35cb4b5b2c..b379dd41bc 100644 --- a/pvlib/tests/test_pvsystem.py +++ b/pvlib/tests/test_pvsystem.py @@ -64,13 +64,15 @@ def test_PVSystem_get_iam_sapm(sapm_module_params, mocker): assert_allclose(out, 1.0, atol=0.01) -def test_PVSystem_get_iam_interp(): - custom_module_params = {'iam_ref': (1., 0.8), 'theta_ref': (0., 80.)} - system = pvsystem.PVSystem(module_parameters=custom_module_params) +def test_PVSystem_get_iam_interp(mocker): + interp_module_params = {'iam_ref': (1., 0.8), 'theta_ref': (0., 80.)} + system = pvsystem.PVSystem(module_parameters=interp_module_params) + spy = mocker.spy(_iam, 'interp') aoi = ((0., 40., 80.),) expected = (1., 0.9, 0.8) out = system.get_iam(aoi, iam_model='interp') assert_allclose(out, expected) + spy.assert_called_once_with(aoi[0], **interp_module_params) def test__normalize_sam_product_names(): From 8febd15848b51d86bbfe190a6b1aff2cccaa94f1 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Tue, 15 Aug 2023 11:02:37 +0200 Subject: [PATCH 05/11] Update test_modelchain.py --- pvlib/tests/test_modelchain.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index d640394cc6..0f6e6dbcd0 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -1467,7 +1467,9 @@ def test_aoi_model_interp(sapm_dc_snl_ac_system, location, weather, mocker): spectral_model='no_loss') m = mocker.spy(iam, 'interp') mc.run_model(weather=weather) - assert m.call_args.kwargs == {'iam_ref': iam_ref, 'theta_ref': theta_ref} + # only test kwargs + assert m.call_args[1]['iam_ref'] == iam_ref + assert m.call_args[1]['theta_ref'] == theta_ref assert isinstance(mc.results.ac, pd.Series) assert not mc.results.ac.empty assert mc.results.ac[0] > 150 and mc.results.ac[0] < 200 From c5e0556df5ea361a596bf4d6fcc4b9eecfec2103 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Fri, 25 Aug 2023 22:44:47 +0200 Subject: [PATCH 06/11] Update error message in modelchain.py Co-Authored-By: Kevin Anderson <57452607+kandersolar@users.noreply.github.com> --- pvlib/modelchain.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index 2d1bd3f790..ae3b323219 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -944,9 +944,9 @@ def infer_aoi_model(self): 'system.arrays[i].module_parameters. Check that ' 'the module_parameters for all Arrays in ' 'system.arrays contain parameters for ' - 'the physical, aoi, ashrae or martin_ruiz model; ' - 'explicitly set the model with the aoi_model ' - 'kwarg; or set aoi_model="no_loss".') + 'the physical, aoi, ashrae, martin_ruiz or interp' + 'model; explicitly set the model with the' + 'aoi_model kwarg; or set aoi_model="no_loss".') def ashrae_aoi_loss(self): self.results.aoi_modifier = self.system.get_iam( From b0795120d8eed63094d6f929fc31fa2bf073b6f1 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 26 Aug 2023 19:26:11 +0200 Subject: [PATCH 07/11] Allow non _IAM_MODEL_PARAMS to be passed to IAM models in pvsystem --- pvlib/pvsystem.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index 80ce503f77..2dcf8f5ee3 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -8,6 +8,7 @@ import io import itertools import os +import inspect from urllib.request import urlopen import numpy as np from scipy import constants @@ -1165,9 +1166,12 @@ def get_iam(self, aoi, iam_model='physical'): """ model = iam_model.lower() if model in ['ashrae', 'physical', 'martin_ruiz', 'interp']: - param_names = iam._IAM_MODEL_PARAMS[model] - kwargs = _build_kwargs(param_names, self.module_parameters) - func = getattr(iam, model) + func = getattr(iam, model) # get function at pvlib.iam + # get all parameters from function signature to retrieve them from + # module_parameters if present + params = set(inspect.signature(func).parameters.keys()) + params.discard('aoi') # exclude aoi so it can't be repeated + kwargs = _build_kwargs(params, self.module_parameters) return func(aoi, **kwargs) elif model == 'sapm': return iam.sapm(aoi, self.module_parameters) From 458dbcea5a12926e6af9d9a485b8b303b1608f15 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Tue, 29 Aug 2023 19:50:50 +0200 Subject: [PATCH 08/11] Add kwargs test --- pvlib/tests/test_modelchain.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index 0f6e6dbcd0..edda3b7e0e 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -1498,6 +1498,26 @@ def test_infer_aoi_model(location, system_no_aoi, aoi_model): assert isinstance(mc, ModelChain) +@pytest.mark.parametrize('aoi_model,model_kwargs', [ + # model_kwargs has both required and optional kwargs; test all + ('physical', + {'n': 1.526, 'K': 4.0, 'L': 0.002, # required + 'n_ar': 1.8}), # extra + ('interp', + {'theta_ref': (0, 75, 85, 90), 'iam_ref': (1, 0.8, 0.42, 0), # required + 'method': 'cubic', 'normalize': False})]) # extra +def test_infer_aoi_model_with_extra_params(location, system_no_aoi, aoi_model, + model_kwargs, weather, mocker): + # test extra parameters not defined at iam._IAM_MODEL_PARAMS are passed + m = mocker.spy(iam, aoi_model) + system_no_aoi.arrays[0].module_parameters.update(**model_kwargs) + mc = ModelChain(system_no_aoi, location, spectral_model='no_loss') + assert isinstance(mc, ModelChain) + mc.run_model(weather=weather) + _, call_kwargs = m.call_args + assert call_kwargs == model_kwargs + + def test_infer_aoi_model_invalid(location, system_no_aoi): exc_text = 'could not infer AOI model' with pytest.raises(ValueError, match=exc_text): From 3db570437c76ad51e183a3c67056bb0b83e0c99e Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Wed, 6 Sep 2023 14:07:37 +0200 Subject: [PATCH 09/11] Update v0.10.2.rst --- docs/sphinx/source/whatsnew/v0.10.2.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/sphinx/source/whatsnew/v0.10.2.rst b/docs/sphinx/source/whatsnew/v0.10.2.rst index f0995041db..6288b077fc 100644 --- a/docs/sphinx/source/whatsnew/v0.10.2.rst +++ b/docs/sphinx/source/whatsnew/v0.10.2.rst @@ -27,6 +27,9 @@ Bug fixes ~~~~~~~~~ * :py:func:`~pvlib.iotools.get_psm3` no longer incorrectly returns clear-sky DHI instead of clear-sky GHI when requesting ``ghi_clear``. (:pull:`1819`) +* :py:class:`pvlib.pvsystem.PVSystem` now correctly passes ``n_ar`` module + parameter to :py:func:`pvlib.iam.physical` when this IAM model is specified + or inferred. (:pull:`1832`) Testing ~~~~~~~ From f544ba4879067e4307b02add84a81f87da86376e Mon Sep 17 00:00:00 2001 From: Echedey Luis <80125792+echedey-ls@users.noreply.github.com> Date: Thu, 7 Sep 2023 00:03:46 +0200 Subject: [PATCH 10/11] Update pvlib/modelchain.py Co-authored-by: Kevin Anderson --- pvlib/modelchain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index ae3b323219..1050ab8115 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -944,8 +944,8 @@ def infer_aoi_model(self): 'system.arrays[i].module_parameters. Check that ' 'the module_parameters for all Arrays in ' 'system.arrays contain parameters for ' - 'the physical, aoi, ashrae, martin_ruiz or interp' - 'model; explicitly set the model with the' + 'the physical, aoi, ashrae, martin_ruiz or interp ' + 'model; explicitly set the model with the ' 'aoi_model kwarg; or set aoi_model="no_loss".') def ashrae_aoi_loss(self): From 4054c66afc0273d17612fd3aaf242c8b0efc8b77 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Thu, 7 Sep 2023 00:07:49 +0200 Subject: [PATCH 11/11] Update modelchain.py --- pvlib/modelchain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index 1050ab8115..389ca68a54 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -943,8 +943,8 @@ def infer_aoi_model(self): raise ValueError('could not infer AOI model from ' 'system.arrays[i].module_parameters. Check that ' 'the module_parameters for all Arrays in ' - 'system.arrays contain parameters for ' - 'the physical, aoi, ashrae, martin_ruiz or interp ' + 'system.arrays contain parameters for the ' + 'physical, aoi, ashrae, martin_ruiz or interp ' 'model; explicitly set the model with the ' 'aoi_model kwarg; or set aoi_model="no_loss".')