From a0ac7a24a6025dd38317db6fef08c08f63c930e5 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Tue, 23 Feb 2021 14:13:50 -0700 Subject: [PATCH 01/12] add attributes --- rdtools/analysis_chains.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/rdtools/analysis_chains.py b/rdtools/analysis_chains.py index d6723ef3..79dd512d 100644 --- a/rdtools/analysis_chains.py +++ b/rdtools/analysis_chains.py @@ -373,7 +373,9 @@ def _filter(self, energy_normalized, case): ------- None ''' - bool_filter = True + filter_components = pd.DataFrame({ + 'default': pd.Series(True, index=energy_normalized.index) + }) if case == 'sensor': poa = self.poa_global @@ -385,19 +387,19 @@ def _filter(self, energy_normalized, case): if 'normalized_filter' in self.filter_params: f = filtering.normalized_filter( energy_normalized, **self.filter_params['normalized_filter']) - bool_filter = bool_filter & f + filter_components['normalized_filter'] = f if 'poa_filter' in self.filter_params: if poa is None: raise ValueError('poa must be available to use poa_filter') f = filtering.poa_filter(poa, **self.filter_params['poa_filter']) - bool_filter = bool_filter & f + filter_components['poa_filter'] = f if 'tcell_filter' in self.filter_params: if cell_temp is None: raise ValueError( 'Cell temperature must be available to use tcell_filter') f = filtering.tcell_filter( cell_temp, **self.filter_params['tcell_filter']) - bool_filter = bool_filter & f + filter_components['tcell_filter'] = f if 'clip_filter' in self.filter_params: if self.pv_power is None: raise ValueError('PV power (not energy) is required for the clipping filter. ' @@ -405,22 +407,26 @@ def _filter(self, energy_normalized, case): 'instantiation, or explicitly assign TrendAnalysis.pv_power.') f = filtering.clip_filter( self.pv_power, **self.filter_params['clip_filter']) - bool_filter = bool_filter & f + filter_components['clip_filter'] = f if 'ad_hoc_filter' in self.filter_params: if self.filter_params['ad_hoc_filter'] is not None: - bool_filter = bool_filter & self.filter_params['ad_hoc_filter'] + filter_components['normalized_filter'] = self.filter_params['ad_hoc_filter'] if case == 'clearsky': if self.poa_global is None or self.poa_global_clearsky is None: raise ValueError('Both poa_global and poa_global_clearsky must be available to do clearsky ' 'filtering with csi_filter') f = filtering.csi_filter( self.poa_global, self.poa_global_clearsky, **self.filter_params['csi_filter']) - bool_filter = bool_filter & f + filter_components['csi_filter'] = f + bool_filter = filter_components.all(axis=1) + filter_components = filter_components.drop(columns=['default']) if case == 'sensor': self.sensor_filter = bool_filter + self.sensor_filter_components = filter_components elif case == 'clearsky': self.clearsky_filter = bool_filter + self.clearsky_filter_components = filter_components def _filter_check(self, post_filter): ''' From cb796592b0c0b0d782b4212055dc1097f7215126 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Tue, 23 Feb 2021 14:13:59 -0700 Subject: [PATCH 02/12] test --- rdtools/test/analysis_chains_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rdtools/test/analysis_chains_test.py b/rdtools/test/analysis_chains_test.py index dbb9577b..ab1e5ac2 100644 --- a/rdtools/test/analysis_chains_test.py +++ b/rdtools/test/analysis_chains_test.py @@ -130,6 +130,15 @@ def test_sensor_analysis_ad_hoc_filter(sensor_parameters): rd_analysis.sensor_analysis(analyses=['yoy_degradation']) +def test_filter_components(sensor_parameters): + poa = sensor_parameters['poa_global'] + poa_filter = (poa > 200) & (poa < 1200) + poa_filter = poa_filter.iloc[1:] # remove first element to align index with expected + rd_analysis = TrendAnalysis(**sensor_parameters, power_dc_rated=1.0) + rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + assert (poa_filter == rd_analysis.sensor_filter_components['poa_filter']).all() + + def test_cell_temperature_model_invalid(sensor_parameters): wind = pd.Series(0, index=sensor_parameters['pv'].index) sensor_parameters.pop('temperature_model') From 7c0042b87b6e9f3a5fc79a3650fe48105c817784 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Tue, 23 Feb 2021 14:14:07 -0700 Subject: [PATCH 03/12] changelog --- docs/sphinx/source/changelog/pending.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/sphinx/source/changelog/pending.rst b/docs/sphinx/source/changelog/pending.rst index 2ad5eb34..3ddf6ca0 100644 --- a/docs/sphinx/source/changelog/pending.rst +++ b/docs/sphinx/source/changelog/pending.rst @@ -7,6 +7,8 @@ Requirements * Update specified versions of bleach in ``docs/notebook_requirements.txt`` and matplotlib in ``requirements.txt`` (:pull:`261`) +* Add ``sensor_filter_components`` and ``clearsky_filter_components`` to + :py:class:`~rdtools.analysis_chains.TrendAnalysis` (:issue:`236`, :pull:`263`) Contributors From 5d54042fd0b1d8b91cb1230c42c255d93d35ab7b Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Tue, 23 Feb 2021 14:33:57 -0700 Subject: [PATCH 04/12] unrelated linting --- rdtools/analysis_chains.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rdtools/analysis_chains.py b/rdtools/analysis_chains.py index 79dd512d..394b0335 100644 --- a/rdtools/analysis_chains.py +++ b/rdtools/analysis_chains.py @@ -413,8 +413,8 @@ def _filter(self, energy_normalized, case): filter_components['normalized_filter'] = self.filter_params['ad_hoc_filter'] if case == 'clearsky': if self.poa_global is None or self.poa_global_clearsky is None: - raise ValueError('Both poa_global and poa_global_clearsky must be available to do clearsky ' - 'filtering with csi_filter') + raise ValueError('Both poa_global and poa_global_clearsky must be available to ' + 'do clearsky filtering with csi_filter') f = filtering.csi_filter( self.poa_global, self.poa_global_clearsky, **self.filter_params['csi_filter']) filter_components['csi_filter'] = f From 77e211a5e370c9b4d8302f99efb19e5e22db208d Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Wed, 24 Feb 2021 15:09:49 -0700 Subject: [PATCH 05/12] bug fixes --- rdtools/analysis_chains.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rdtools/analysis_chains.py b/rdtools/analysis_chains.py index 394b0335..a6e14632 100644 --- a/rdtools/analysis_chains.py +++ b/rdtools/analysis_chains.py @@ -410,7 +410,7 @@ def _filter(self, energy_normalized, case): filter_components['clip_filter'] = f if 'ad_hoc_filter' in self.filter_params: if self.filter_params['ad_hoc_filter'] is not None: - filter_components['normalized_filter'] = self.filter_params['ad_hoc_filter'] + filter_components['ad_hoc_filter'] = self.filter_params['ad_hoc_filter'] if case == 'clearsky': if self.poa_global is None or self.poa_global_clearsky is None: raise ValueError('Both poa_global and poa_global_clearsky must be available to ' @@ -419,7 +419,9 @@ def _filter(self, energy_normalized, case): self.poa_global, self.poa_global_clearsky, **self.filter_params['csi_filter']) filter_components['csi_filter'] = f - bool_filter = filter_components.all(axis=1) + # note: the previous implementation using the & operator treated NaN + # filter values as False, so we do the same here for consistency: + bool_filter = filter_components.fillna(False).all(axis=1) filter_components = filter_components.drop(columns=['default']) if case == 'sensor': self.sensor_filter = bool_filter From 7241b7c167d6cfc62b7eda3c57243083cb0009dd Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Wed, 21 Apr 2021 14:10:49 -0600 Subject: [PATCH 06/12] remove unnecessary default filter, test --- rdtools/analysis_chains.py | 5 +---- rdtools/test/analysis_chains_test.py | 9 +++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/rdtools/analysis_chains.py b/rdtools/analysis_chains.py index a6e14632..1f72f680 100644 --- a/rdtools/analysis_chains.py +++ b/rdtools/analysis_chains.py @@ -373,9 +373,7 @@ def _filter(self, energy_normalized, case): ------- None ''' - filter_components = pd.DataFrame({ - 'default': pd.Series(True, index=energy_normalized.index) - }) + filter_components = pd.DataFrame(index=energy_normalized.index) if case == 'sensor': poa = self.poa_global @@ -422,7 +420,6 @@ def _filter(self, energy_normalized, case): # note: the previous implementation using the & operator treated NaN # filter values as False, so we do the same here for consistency: bool_filter = filter_components.fillna(False).all(axis=1) - filter_components = filter_components.drop(columns=['default']) if case == 'sensor': self.sensor_filter = bool_filter self.sensor_filter_components = filter_components diff --git a/rdtools/test/analysis_chains_test.py b/rdtools/test/analysis_chains_test.py index ab1e5ac2..c73741e1 100644 --- a/rdtools/test/analysis_chains_test.py +++ b/rdtools/test/analysis_chains_test.py @@ -139,6 +139,15 @@ def test_filter_components(sensor_parameters): assert (poa_filter == rd_analysis.sensor_filter_components['poa_filter']).all() +def test_filter_components_no_filters(sensor_parameters): + rd_analysis = TrendAnalysis(**sensor_parameters, power_dc_rated=1.0) + rd_analysis.filter_params = {} # disable all filters + rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + expected = pd.Series(True, index=rd_analysis.pv_energy.index) + pd.testing.assert_series_equal(rd_analysis.sensor_filter, expected) + assert rd_analysis.sensor_filter_components.empty + + def test_cell_temperature_model_invalid(sensor_parameters): wind = pd.Series(0, index=sensor_parameters['pv'].index) sensor_parameters.pop('temperature_model') From c883fb0b04c31c913ab8b95a3993478283a632d7 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Thu, 22 Apr 2021 18:12:41 -0600 Subject: [PATCH 07/12] fix up index union funny business --- rdtools/analysis_chains.py | 13 +++++++++++-- rdtools/test/analysis_chains_test.py | 1 - 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/rdtools/analysis_chains.py b/rdtools/analysis_chains.py index 1f72f680..49a22090 100644 --- a/rdtools/analysis_chains.py +++ b/rdtools/analysis_chains.py @@ -373,7 +373,14 @@ def _filter(self, energy_normalized, case): ------- None ''' - filter_components = pd.DataFrame(index=energy_normalized.index) + # Combining filters is non-trivial because of the possibility of index + # mismatch. Adding columns to an existing dataframe performs a left index + # join, but probably we actually want an outer join. We can get an outer + # join by keeping this as a dictionary and converting it to a dataframe all + # at once. However, we add a default value of True, with the same index as + # energy_normalized, so that the output is still correct even when all + # filters have been disabled. + filter_components = {'default': pd.Series(True, index=energy_normalized.index)} if case == 'sensor': poa = self.poa_global @@ -419,7 +426,9 @@ def _filter(self, energy_normalized, case): # note: the previous implementation using the & operator treated NaN # filter values as False, so we do the same here for consistency: - bool_filter = filter_components.fillna(False).all(axis=1) + filter_components = pd.DataFrame(filter_components).fillna(False) + bool_filter = filter_components.all(axis=1) + filter_components = filter_components.drop(columns=['default']) if case == 'sensor': self.sensor_filter = bool_filter self.sensor_filter_components = filter_components diff --git a/rdtools/test/analysis_chains_test.py b/rdtools/test/analysis_chains_test.py index c73741e1..df1a83e8 100644 --- a/rdtools/test/analysis_chains_test.py +++ b/rdtools/test/analysis_chains_test.py @@ -133,7 +133,6 @@ def test_sensor_analysis_ad_hoc_filter(sensor_parameters): def test_filter_components(sensor_parameters): poa = sensor_parameters['poa_global'] poa_filter = (poa > 200) & (poa < 1200) - poa_filter = poa_filter.iloc[1:] # remove first element to align index with expected rd_analysis = TrendAnalysis(**sensor_parameters, power_dc_rated=1.0) rd_analysis.sensor_analysis(analyses=['yoy_degradation']) assert (poa_filter == rd_analysis.sensor_filter_components['poa_filter']).all() From bd63871f2f38869cd58143053105e5598f58e340 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Tue, 25 May 2021 16:24:32 -0600 Subject: [PATCH 08/12] preliminary fix for #266 --- rdtools/analysis_chains.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/rdtools/analysis_chains.py b/rdtools/analysis_chains.py index 9528de29..fb62a601 100644 --- a/rdtools/analysis_chains.py +++ b/rdtools/analysis_chains.py @@ -413,9 +413,6 @@ def _filter(self, energy_normalized, case): f = filtering.clip_filter( self.pv_power, **self.filter_params['clip_filter']) filter_components['clip_filter'] = f - if 'ad_hoc_filter' in self.filter_params: - if self.filter_params['ad_hoc_filter'] is not None: - filter_components['ad_hoc_filter'] = self.filter_params['ad_hoc_filter'] if case == 'clearsky': if self.poa_global is None or self.poa_global_clearsky is None: raise ValueError('Both poa_global and poa_global_clearsky must be available to ' @@ -427,6 +424,23 @@ def _filter(self, energy_normalized, case): # note: the previous implementation using the & operator treated NaN # filter values as False, so we do the same here for consistency: filter_components = pd.DataFrame(filter_components).fillna(False) + + # apply special checks to ad_hoc_filter, as it is likely more prone to user error + if self.filter_params.get('ad_hoc_filter', None) is not None: + ad_hoc_filter = self.filter_params['ad_hoc_filter'] + + if ad_hoc_filter.isnull().any(): + warnings.warn('ad_hoc_filter contains NaN values; setting to False') + ad_hoc_filter = ad_hoc_filter.fillna(False) + + if not filter_components.index.equals(ad_hoc_filter.index): + warnings.warn('ad_hoc_filter index does not match index of other filters; missing ' + 'values will be set to True. Align the index with the index of the ' + 'filter_components attribute to prevent this warning') + ad_hoc_filter = ad_hoc_filter.reindex(filter_components.index).fillna(True) + + filter_components['ad_hoc_filter'] = ad_hoc_filter + bool_filter = filter_components.all(axis=1) filter_components = filter_components.drop(columns=['default']) if case == 'sensor': From a331e0f73ca39050606f2a81af16120e744db572 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Thu, 27 May 2021 15:38:11 -0600 Subject: [PATCH 09/12] add test to cover new warnings --- rdtools/test/analysis_chains_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/rdtools/test/analysis_chains_test.py b/rdtools/test/analysis_chains_test.py index 9cec15bc..c3516a64 100644 --- a/rdtools/test/analysis_chains_test.py +++ b/rdtools/test/analysis_chains_test.py @@ -148,6 +148,22 @@ def test_filter_components_no_filters(sensor_parameters): assert rd_analysis.sensor_filter_components.empty +def test_filter_ad_hoc_warnings(sensor_parameters): + rd_analysis = TrendAnalysis(**sensor_parameters, power_dc_rated=1.0) + # warning for incomplete index + ad_hoc_filter = pd.Series(True, index=sensor_parameters['pv'].index[:-5]) + rd_analysis.filter_params['ad_hoc_filter'] = ad_hoc_filter + with pytest.warns(UserWarning, match='ad_hoc_filter index does not match index'): + rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + + # warning about NaNs + ad_hoc_filter = pd.Series(True, index=sensor_parameters['pv']) + ad_hoc_filter.iloc[10] = np.nan + rd_analysis.filter_params['ad_hoc_filter'] = ad_hoc_filter + with pytest.warns(UserWarning, match='ad_hoc_filter contains NaN values; setting to False'): + rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + + def test_cell_temperature_model_invalid(sensor_parameters): wind = pd.Series(0, index=sensor_parameters['pv'].index) sensor_parameters.pop('temperature_model') From 002fc2ca976a6b3ee030f14abcfaa9e170bf8950 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Fri, 28 May 2021 11:01:19 -0600 Subject: [PATCH 10/12] improve warning messages --- rdtools/analysis_chains.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rdtools/analysis_chains.py b/rdtools/analysis_chains.py index fb62a601..bab140ad 100644 --- a/rdtools/analysis_chains.py +++ b/rdtools/analysis_chains.py @@ -430,13 +430,13 @@ def _filter(self, energy_normalized, case): ad_hoc_filter = self.filter_params['ad_hoc_filter'] if ad_hoc_filter.isnull().any(): - warnings.warn('ad_hoc_filter contains NaN values; setting to False') + warnings.warn('ad_hoc_filter contains NaN values; setting to False (excluding)') ad_hoc_filter = ad_hoc_filter.fillna(False) if not filter_components.index.equals(ad_hoc_filter.index): warnings.warn('ad_hoc_filter index does not match index of other filters; missing ' - 'values will be set to True. Align the index with the index of the ' - 'filter_components attribute to prevent this warning') + 'values will be set to True (kept). Align the index with the index ' + 'of the filter_components attribute to prevent this warning') ad_hoc_filter = ad_hoc_filter.reindex(filter_components.index).fillna(True) filter_components['ad_hoc_filter'] = ad_hoc_filter From 6dac0302ee3e76f47bab48bac80aa0b64d88a939 Mon Sep 17 00:00:00 2001 From: Kevin Anderson Date: Fri, 28 May 2021 11:01:31 -0600 Subject: [PATCH 11/12] expand tests --- rdtools/test/analysis_chains_test.py | 30 ++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/rdtools/test/analysis_chains_test.py b/rdtools/test/analysis_chains_test.py index c3516a64..39ce52fa 100644 --- a/rdtools/test/analysis_chains_test.py +++ b/rdtools/test/analysis_chains_test.py @@ -148,20 +148,42 @@ def test_filter_components_no_filters(sensor_parameters): assert rd_analysis.sensor_filter_components.empty -def test_filter_ad_hoc_warnings(sensor_parameters): +@pytest.mark.parametrize('workflow', ['sensor', 'clearsky']) +def test_filter_ad_hoc_warnings(workflow, sensor_parameters): rd_analysis = TrendAnalysis(**sensor_parameters, power_dc_rated=1.0) + rd_analysis.set_clearsky(pvlib_location=pvlib.location.Location(40, -80), + poa_global_clearsky=rd_analysis.poa_global) + # warning for incomplete index ad_hoc_filter = pd.Series(True, index=sensor_parameters['pv'].index[:-5]) rd_analysis.filter_params['ad_hoc_filter'] = ad_hoc_filter with pytest.warns(UserWarning, match='ad_hoc_filter index does not match index'): - rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + if workflow == 'sensor': + rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + components = rd_analysis.sensor_filter_components + else: + rd_analysis.clearsky_analysis(analyses=['yoy_degradation']) + components = rd_analysis.clearsky_filter_components + + # missing values set to True + assert components['ad_hoc_filter'].all() # warning about NaNs - ad_hoc_filter = pd.Series(True, index=sensor_parameters['pv']) + ad_hoc_filter = pd.Series(True, index=sensor_parameters['pv'].index) ad_hoc_filter.iloc[10] = np.nan rd_analysis.filter_params['ad_hoc_filter'] = ad_hoc_filter with pytest.warns(UserWarning, match='ad_hoc_filter contains NaN values; setting to False'): - rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + if workflow == 'sensor': + rd_analysis.sensor_analysis(analyses=['yoy_degradation']) + components = rd_analysis.sensor_filter_components + else: + rd_analysis.clearsky_analysis(analyses=['yoy_degradation']) + components = rd_analysis.clearsky_filter_components + + # NaN values set to False + assert not components['ad_hoc_filter'].iloc[10] + assert components.drop(components.index[10])['ad_hoc_filter'].all() + def test_cell_temperature_model_invalid(sensor_parameters): From 03507bc9be369a5ab06d4a8fa13dd0e8a2e9b155 Mon Sep 17 00:00:00 2001 From: Michael Deceglie Date: Fri, 28 May 2021 14:00:38 -0600 Subject: [PATCH 12/12] delete extra blank line --- rdtools/test/analysis_chains_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rdtools/test/analysis_chains_test.py b/rdtools/test/analysis_chains_test.py index 2eaae2f4..511613be 100644 --- a/rdtools/test/analysis_chains_test.py +++ b/rdtools/test/analysis_chains_test.py @@ -185,7 +185,6 @@ def test_filter_ad_hoc_warnings(workflow, sensor_parameters): assert components.drop(components.index[10])['ad_hoc_filter'].all() - def test_cell_temperature_model_invalid(sensor_parameters): wind = pd.Series(0, index=sensor_parameters['pv'].index) sensor_parameters.pop('temperature_model')