From 0eea3a1a790f5839bd6b5790e15ce15aca4938bf Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Sat, 6 Feb 2021 10:13:48 -0700 Subject: [PATCH 1/8] check for complete poa in _set_celltemp --- pvlib/modelchain.py | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index 6a8903be85..97e8748967 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -1051,6 +1051,18 @@ def _set_celltemp(self, model): poa = _irrad_for_celltemp(self.results.total_irrad, self.results.effective_irradiance) + # Because this operates on all Arrays simultaneously, poa must be known + # for all arrays. + if any(p is None for p in poa): + # Provide a more informative error message. Because only + # run_model_from_effective_irradiance() can get to this point + # without known POA we can suggest a very specific remedy in the + # error message. + raise ValueError("Incomplete input data. Data must contain " + "'poa_global' for every Array, or must contain " + "'effective_irradiance' for every Array, " + "including Arrays that have a known " + "'cell_temperature' or 'module_temperature'.") temp_air = _tuple_from_dfs(self.weather, 'temp_air') wind_speed = _tuple_from_dfs(self.weather, 'wind_speed') self.results.cell_temperature = model(poa, temp_air, wind_speed) @@ -1572,11 +1584,13 @@ def _prepare_temperature(self, data=None): """ poa = _irrad_for_celltemp(self.results.total_irrad, self.results.effective_irradiance) - if not isinstance(data, tuple) and self.system.num_arrays > 1: + # handle simple case first, single array, data not iterable + if not isinstance(data, tuple) and self.system.num_arrays == 1: + return self._prepare_temperature_single_array(data, poa) + if not isinstance(data, tuple): # broadcast data to all arrays data = (data,) * self.system.num_arrays - elif not isinstance(data, tuple): - return self._prepare_temperature_single_array(data, poa) + # find where cell or module temperature is specified in input data given_cell_temperature = tuple(itertools.starmap( self._get_cell_temperature, zip(data, poa, self.system.temperature_model_parameters) @@ -1588,22 +1602,7 @@ def _prepare_temperature(self, data=None): return self # Calculate cell temperature from weather data. If cell_temperature # has not been provided for some arrays then it is computed with - # ModelChain.temperature_model(). Because this operates on all Arrays - # simultaneously, 'poa_global' must be known for all arrays, including - # those that have a known cell temperature. - try: - self._verify_df(self.results.total_irrad, ['poa_global']) - except ValueError: - # Provide a more informative error message. Because only - # run_model_from_effective_irradiance() can get to this point - # without known POA we can suggest a very specific remedy in the - # error message. - raise ValueError("Incomplete input data. Data must contain " - "'poa_global'. For systems with multiple Arrays " - "if you have provided 'cell_temperature' for " - "only a subset of Arrays you must provide " - "'poa_global' for all Arrays, including those " - "that have a known 'cell_temperature'.") + # ModelChain.temperature_model(). self.temperature_model() # replace calculated cell temperature with temperature given in `data` # where available. From 27ed23a6120dd4f3804dccd4bcf5a41c78b4b787 Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Sat, 6 Feb 2021 10:58:06 -0700 Subject: [PATCH 2/8] test for new location of ValueError, add test for run_from_eff_irrad with iterable of length 1 --- pvlib/tests/test_modelchain.py | 35 +++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index 1e627b541b..9ef2487653 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -910,6 +910,20 @@ def test_temperature_models_arrays_multi_weather( != mc.results.cell_temperature[1]).all() +def test__set_celltemp_missing_poa( + sapm_dc_snl_ac_system_same_arrays, location, weather, total_irrad): + mc = ModelChain(sapm_dc_snl_ac_system_same_arrays, location, + aoi_model='no_loss', spectral_model='no_loss') + data_one = total_irrad + data_two = total_irrad.copy() + data_one['effective_irradiance'] = data_one['poa_global'] + data_two.pop('poa_global') + matchtxt = "Incomplete input data. Data must contain 'poa_global' " \ + "for every Array" + with pytest.raises(ValueError, match=matchtxt): + mc.run_model_from_effective_irradiance((data_one, data_two)) + + def test_run_model_solar_position_weather( pvwatts_dc_pvwatts_ac_system, location, weather, mocker): mc = ModelChain(pvwatts_dc_pvwatts_ac_system, location, @@ -985,8 +999,9 @@ def test_run_model_from_poa_tracking(sapm_dc_snl_ac_system, location, assert_series_equal(ac, expected) +@pytest.mark.parametrize("input_type", [tuple, list]) def test_run_model_from_effective_irradiance(sapm_dc_snl_ac_system, location, - weather, total_irrad): + weather, total_irrad, input_type): data = weather.copy() data[['poa_global', 'poa_diffuse', 'poa_direct']] = total_irrad data['effective_irradiance'] = data['poa_global'] @@ -996,6 +1011,24 @@ def test_run_model_from_effective_irradiance(sapm_dc_snl_ac_system, location, expected = pd.Series(np.array([149.280238, 96.678385]), index=data.index) assert_series_equal(ac, expected) + # check with data as an iterable of length 1 + mc.run_model_from_effective_irradiance(input_type(data)) + + +@pytest.mark.parametrize("input_type", [tuple, list]) +def test_run_model_from_effective_irradiance_input_type( + sapm_dc_snl_ac_system_Array, location, weather, total_irrad, + input_type): + data = weather.copy() + data[['poa_global', 'poa_diffuse', 'poa_direct']] = total_irrad + data['effective_irradiance'] = data['poa_global'] + mc = ModelChain(sapm_dc_snl_ac_system_Array, location, aoi_model='no_loss', + spectral_model='no_loss') + mc.run_model_from_effective_irradiance(input_type((data, data))) + # arrays have different orientation, but should give same dc power + # because we are the same passing POA irradiance and air + # temperature. + assert_frame_equal(mc.results.dc[0], mc.results.dc[1]) def test_run_model_from_effective_irradiance_no_poa_global( From 94ca10693d17c308aafcd5848e83c36bdd09f56b Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Sat, 6 Feb 2021 11:32:23 -0700 Subject: [PATCH 3/8] handle single data/arrays, fix test --- pvlib/modelchain.py | 2 +- pvlib/tests/test_modelchain.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index 97e8748967..c6fd94e775 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -1053,7 +1053,7 @@ def _set_celltemp(self, model): self.results.effective_irradiance) # Because this operates on all Arrays simultaneously, poa must be known # for all arrays. - if any(p is None for p in poa): + if poa is None or any(p is None for p in poa): # Provide a more informative error message. Because only # run_model_from_effective_irradiance() can get to this point # without known POA we can suggest a very specific remedy in the diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index 9ef2487653..0f1fcb7a0f 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -916,12 +916,12 @@ def test__set_celltemp_missing_poa( aoi_model='no_loss', spectral_model='no_loss') data_one = total_irrad data_two = total_irrad.copy() - data_one['effective_irradiance'] = data_one['poa_global'] data_two.pop('poa_global') + mc._assign_total_irrad((data_one, data_two)) matchtxt = "Incomplete input data. Data must contain 'poa_global' " \ "for every Array" with pytest.raises(ValueError, match=matchtxt): - mc.run_model_from_effective_irradiance((data_one, data_two)) + mc._set_celltemp('sapm') def test_run_model_solar_position_weather( @@ -1012,7 +1012,7 @@ def test_run_model_from_effective_irradiance(sapm_dc_snl_ac_system, location, index=data.index) assert_series_equal(ac, expected) # check with data as an iterable of length 1 - mc.run_model_from_effective_irradiance(input_type(data)) + mc.run_model_from_effective_irradiance(input_type((data,))) @pytest.mark.parametrize("input_type", [tuple, list]) From 2d20a386c5be95337c0724c2c0a43fd2d29507e4 Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Mon, 8 Feb 2021 09:14:17 -0700 Subject: [PATCH 4/8] remove old test --- pvlib/tests/test_modelchain.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index 0f1fcb7a0f..e45f463997 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -1120,22 +1120,6 @@ def test_run_model_from_effective_irradiance_minimal_input( assert not mc.results.ac.empty -def test_run_model_from_effective_irradiance_missing_poa( - sapm_dc_snl_ac_system_Array, location, total_irrad): - data_incomplete = pd.DataFrame( - {'effective_irradiance': total_irrad['poa_global'], - 'poa_global': total_irrad['poa_global']}, - index=total_irrad.index) - data_complete = pd.DataFrame( - {'effective_irradiance': total_irrad['poa_global'], - 'cell_temperature': 30}, - index=total_irrad.index) - mc = ModelChain(sapm_dc_snl_ac_system_Array, location) - with pytest.raises(ValueError, - match="you must provide 'poa_global' for all Arrays"): - mc.run_model_from_effective_irradiance( - (data_complete, data_incomplete)) - def test_run_model_singleton_weather_single_array(cec_dc_snl_ac_system, location, weather): From aa750e16cfc65f77ad8ccb0857639c1a16f9bb4f Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Mon, 8 Feb 2021 10:36:35 -0700 Subject: [PATCH 5/8] add _verify_df to run_from_effective_irradiance --- pvlib/modelchain.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index c6fd94e775..2d5634345d 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -1813,6 +1813,7 @@ def run_model_from_effective_irradiance(self, data=None): """ data = _to_tuple(data) self._check_multiple_input(data) + self._verify_df(data, required=['effective_irradiance']) self._assign_weather(data) self._assign_total_irrad(data) self.results.effective_irradiance = _tuple_from_dfs( From df5b8fd3c7fc081d9323ac1ccf8efe78ed95870b Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Mon, 8 Feb 2021 10:37:56 -0700 Subject: [PATCH 6/8] remove ValueError from _set_celltemp --- pvlib/modelchain.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index 2d5634345d..69ec94c128 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -1051,18 +1051,6 @@ def _set_celltemp(self, model): poa = _irrad_for_celltemp(self.results.total_irrad, self.results.effective_irradiance) - # Because this operates on all Arrays simultaneously, poa must be known - # for all arrays. - if poa is None or any(p is None for p in poa): - # Provide a more informative error message. Because only - # run_model_from_effective_irradiance() can get to this point - # without known POA we can suggest a very specific remedy in the - # error message. - raise ValueError("Incomplete input data. Data must contain " - "'poa_global' for every Array, or must contain " - "'effective_irradiance' for every Array, " - "including Arrays that have a known " - "'cell_temperature' or 'module_temperature'.") temp_air = _tuple_from_dfs(self.weather, 'temp_air') wind_speed = _tuple_from_dfs(self.weather, 'wind_speed') self.results.cell_temperature = model(poa, temp_air, wind_speed) From 00eb76f0507dca1b006699746dc9aa7159029b2d Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Mon, 8 Feb 2021 10:43:29 -0700 Subject: [PATCH 7/8] test for single, list tuple --- pvlib/tests/test_modelchain.py | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index e45f463997..c22a259f67 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -910,20 +910,6 @@ def test_temperature_models_arrays_multi_weather( != mc.results.cell_temperature[1]).all() -def test__set_celltemp_missing_poa( - sapm_dc_snl_ac_system_same_arrays, location, weather, total_irrad): - mc = ModelChain(sapm_dc_snl_ac_system_same_arrays, location, - aoi_model='no_loss', spectral_model='no_loss') - data_one = total_irrad - data_two = total_irrad.copy() - data_two.pop('poa_global') - mc._assign_total_irrad((data_one, data_two)) - matchtxt = "Incomplete input data. Data must contain 'poa_global' " \ - "for every Array" - with pytest.raises(ValueError, match=matchtxt): - mc._set_celltemp('sapm') - - def test_run_model_solar_position_weather( pvwatts_dc_pvwatts_ac_system, location, weather, mocker): mc = ModelChain(pvwatts_dc_pvwatts_ac_system, location, @@ -999,7 +985,7 @@ def test_run_model_from_poa_tracking(sapm_dc_snl_ac_system, location, assert_series_equal(ac, expected) -@pytest.mark.parametrize("input_type", [tuple, list]) +@pytest.mark.parametrize("input_type", [lambda x: x[0], tuple, list]) def test_run_model_from_effective_irradiance(sapm_dc_snl_ac_system, location, weather, total_irrad, input_type): data = weather.copy() @@ -1007,7 +993,7 @@ def test_run_model_from_effective_irradiance(sapm_dc_snl_ac_system, location, data['effective_irradiance'] = data['poa_global'] mc = ModelChain(sapm_dc_snl_ac_system, location, aoi_model='no_loss', spectral_model='no_loss') - ac = mc.run_model_from_effective_irradiance(data).results.ac + ac = mc.run_model_from_effective_irradiance(input_type((data,))).results.ac expected = pd.Series(np.array([149.280238, 96.678385]), index=data.index) assert_series_equal(ac, expected) @@ -1031,13 +1017,14 @@ def test_run_model_from_effective_irradiance_input_type( assert_frame_equal(mc.results.dc[0], mc.results.dc[1]) +@pytest.mark.parametrize("input_type", [lambda x: x[0], tuple, list]) def test_run_model_from_effective_irradiance_no_poa_global( - sapm_dc_snl_ac_system, location, weather, total_irrad): + sapm_dc_snl_ac_system, location, weather, total_irrad, input_type): data = weather.copy() data['effective_irradiance'] = total_irrad['poa_global'] mc = ModelChain(sapm_dc_snl_ac_system, location, aoi_model='no_loss', spectral_model='no_loss') - ac = mc.run_model_from_effective_irradiance(data).results.ac + ac = mc.run_model_from_effective_irradiance(input_type((data,))).results.ac expected = pd.Series(np.array([149.280238, 96.678385]), index=data.index) assert_series_equal(ac, expected) From a3aa9249f5b89348eb640e6a57fac0451e98d267 Mon Sep 17 00:00:00 2001 From: Cliff Hansen Date: Mon, 8 Feb 2021 11:18:35 -0700 Subject: [PATCH 8/8] edits from review --- pvlib/modelchain.py | 3 +-- pvlib/tests/test_modelchain.py | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pvlib/modelchain.py b/pvlib/modelchain.py index 69ec94c128..1c07857745 100644 --- a/pvlib/modelchain.py +++ b/pvlib/modelchain.py @@ -1589,8 +1589,7 @@ def _prepare_temperature(self, data=None): self.results.cell_temperature = given_cell_temperature return self # Calculate cell temperature from weather data. If cell_temperature - # has not been provided for some arrays then it is computed with - # ModelChain.temperature_model(). + # has not been provided for some arrays then it is computed. self.temperature_model() # replace calculated cell temperature with temperature given in `data` # where available. diff --git a/pvlib/tests/test_modelchain.py b/pvlib/tests/test_modelchain.py index c22a259f67..c45fd5026f 100644 --- a/pvlib/tests/test_modelchain.py +++ b/pvlib/tests/test_modelchain.py @@ -997,12 +997,10 @@ def test_run_model_from_effective_irradiance(sapm_dc_snl_ac_system, location, expected = pd.Series(np.array([149.280238, 96.678385]), index=data.index) assert_series_equal(ac, expected) - # check with data as an iterable of length 1 - mc.run_model_from_effective_irradiance(input_type((data,))) @pytest.mark.parametrize("input_type", [tuple, list]) -def test_run_model_from_effective_irradiance_input_type( +def test_run_model_from_effective_irradiance_multi_array( sapm_dc_snl_ac_system_Array, location, weather, total_irrad, input_type): data = weather.copy() @@ -1107,7 +1105,6 @@ def test_run_model_from_effective_irradiance_minimal_input( assert not mc.results.ac.empty - def test_run_model_singleton_weather_single_array(cec_dc_snl_ac_system, location, weather): mc = ModelChain(cec_dc_snl_ac_system, location,