From 94259c59027e787f683a6c47a364b2db17af5ed3 Mon Sep 17 00:00:00 2001 From: Andrew Godbehere Date: Thu, 19 Sep 2024 15:50:38 -0400 Subject: [PATCH 1/5] Implements closed-form solution for optimization problem in clearsky. Updates test. --- docs/sphinx/source/whatsnew/v0.11.1.rst | 5 +++++ pvlib/clearsky.py | 28 ++++++------------------ pvlib/tests/test_clearsky.py | 29 ++++++++++++------------- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/docs/sphinx/source/whatsnew/v0.11.1.rst b/docs/sphinx/source/whatsnew/v0.11.1.rst index 7c8f01743e..781a68f32f 100644 --- a/docs/sphinx/source/whatsnew/v0.11.1.rst +++ b/docs/sphinx/source/whatsnew/v0.11.1.rst @@ -30,6 +30,10 @@ Enhancements * Added function for calculating wind speed at different heights, :py:func:`pvlib.atmosphere.windspeed_powerlaw`. (:issue:`2118`, :pull:`2124`) +* Implemented closed-form solution for alpha in detect_clearsky, obviating + the call to scipy.optimize that was prone to runtime errors and minimizing + computation, :py:func:`pvlib.clearsky.detect_clearsky`. Resolves :issue:`2712`. + Bug fixes ~~~~~~~~~ @@ -74,3 +78,4 @@ Contributors * Marcos R. Escudero (:ghuser:`marc-resc`) * Bernat Nicolau (:ghuser:`BernatNicolau`) * Eduardo Sarquis (:ghuser:`EduardoSarquis`) +* Andrew B Godbehere (:ghuser:`agodbehere`) \ No newline at end of file diff --git a/pvlib/clearsky.py b/pvlib/clearsky.py index 62d83bc1f4..3b36262dda 100644 --- a/pvlib/clearsky.py +++ b/pvlib/clearsky.py @@ -3,15 +3,14 @@ to calculate clear sky GHI, DNI, and DHI. """ +import calendar import os from collections import OrderedDict -import calendar +import h5py import numpy as np import pandas as pd -from scipy.optimize import minimize_scalar from scipy.linalg import hankel -import h5py from pvlib import atmosphere, tools from pvlib.tools import _degrees_to_index @@ -874,24 +873,11 @@ def detect_clearsky(measured, clearsky, times=None, infer_limits=False, clear_meas = meas[clear_samples] clear_clear = clear[clear_samples] - def rmse(alpha): - return np.sqrt(np.mean((clear_meas - alpha*clear_clear)**2)) - - optimize_result = minimize_scalar(rmse) - if not optimize_result.success: - try: - message = "Optimizer exited unsuccessfully: " \ - + optimize_result.message - except AttributeError: - message = "Optimizer exited unsuccessfully: \ - No message explaining the failure was returned. \ - If you would like to see this message, please \ - update your scipy version (try version 1.8.0 \ - or beyond)." - raise RuntimeError(message) - - else: - alpha = optimize_result.x + # Compute arg min of MSE between model and observations + C = (clear_clear**2).sum() + if not (pd.isna(C) or C == 0): # safety check + # only update alpha if C is strictly positive + alpha = (clear_meas * clear_clear).sum() / C if round(alpha*10000) == round(previous_alpha*10000): break diff --git a/pvlib/tests/test_clearsky.py b/pvlib/tests/test_clearsky.py index 4e353e997c..79502db01c 100644 --- a/pvlib/tests/test_clearsky.py +++ b/pvlib/tests/test_clearsky.py @@ -1,22 +1,17 @@ from collections import OrderedDict import numpy as np -from numpy import nan import pandas as pd -import pytz -from scipy.linalg import hankel - import pytest +import pytz +from numpy import nan from numpy.testing import assert_allclose -from .conftest import assert_frame_equal, assert_series_equal +from scipy.linalg import hankel +from pvlib import atmosphere, clearsky, irradiance, solarposition from pvlib.location import Location -from pvlib import clearsky -from pvlib import solarposition -from pvlib import atmosphere -from pvlib import irradiance -from .conftest import DATA_DIR +from .conftest import DATA_DIR, assert_frame_equal, assert_series_equal def test_ineichen_series(): @@ -677,12 +672,16 @@ def test_detect_clearsky_not_enough_data(detect_clearsky_data): with pytest.raises(ValueError, match='have at least'): clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=60) - -def test_detect_clearsky_optimizer_failed(detect_clearsky_data): +@pytest.mark.parametrize("window_length", [5, 10, 15, 20, 25]) +def test_detect_clearsky_optimizer_not_failed( + detect_clearsky_data, + window_length + ): expected, cs = detect_clearsky_data - with pytest.raises(RuntimeError, match='Optimizer exited unsuccessfully'): - clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=15) - + clear_samples = clearsky.detect_clearsky( + expected["GHI"], cs["ghi"], window_length=window_length + ) + assert isinstance(clear_samples, pd.Series) @pytest.fixture def detect_clearsky_helper_data(): From 862a28dafb15c3027d9c139b1fa061ec906636f4 Mon Sep 17 00:00:00 2001 From: Andrew Godbehere Date: Thu, 19 Sep 2024 15:52:25 -0400 Subject: [PATCH 2/5] Revert "Implements closed-form solution for optimization problem in clearsky. Updates test." This reverts commit 94259c59027e787f683a6c47a364b2db17af5ed3. --- docs/sphinx/source/whatsnew/v0.11.1.rst | 5 ----- pvlib/clearsky.py | 28 ++++++++++++++++++------ pvlib/tests/test_clearsky.py | 29 +++++++++++++------------ 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/docs/sphinx/source/whatsnew/v0.11.1.rst b/docs/sphinx/source/whatsnew/v0.11.1.rst index 781a68f32f..7c8f01743e 100644 --- a/docs/sphinx/source/whatsnew/v0.11.1.rst +++ b/docs/sphinx/source/whatsnew/v0.11.1.rst @@ -30,10 +30,6 @@ Enhancements * Added function for calculating wind speed at different heights, :py:func:`pvlib.atmosphere.windspeed_powerlaw`. (:issue:`2118`, :pull:`2124`) -* Implemented closed-form solution for alpha in detect_clearsky, obviating - the call to scipy.optimize that was prone to runtime errors and minimizing - computation, :py:func:`pvlib.clearsky.detect_clearsky`. Resolves :issue:`2712`. - Bug fixes ~~~~~~~~~ @@ -78,4 +74,3 @@ Contributors * Marcos R. Escudero (:ghuser:`marc-resc`) * Bernat Nicolau (:ghuser:`BernatNicolau`) * Eduardo Sarquis (:ghuser:`EduardoSarquis`) -* Andrew B Godbehere (:ghuser:`agodbehere`) \ No newline at end of file diff --git a/pvlib/clearsky.py b/pvlib/clearsky.py index 3b36262dda..62d83bc1f4 100644 --- a/pvlib/clearsky.py +++ b/pvlib/clearsky.py @@ -3,14 +3,15 @@ to calculate clear sky GHI, DNI, and DHI. """ -import calendar import os from collections import OrderedDict +import calendar -import h5py import numpy as np import pandas as pd +from scipy.optimize import minimize_scalar from scipy.linalg import hankel +import h5py from pvlib import atmosphere, tools from pvlib.tools import _degrees_to_index @@ -873,11 +874,24 @@ def detect_clearsky(measured, clearsky, times=None, infer_limits=False, clear_meas = meas[clear_samples] clear_clear = clear[clear_samples] - # Compute arg min of MSE between model and observations - C = (clear_clear**2).sum() - if not (pd.isna(C) or C == 0): # safety check - # only update alpha if C is strictly positive - alpha = (clear_meas * clear_clear).sum() / C + def rmse(alpha): + return np.sqrt(np.mean((clear_meas - alpha*clear_clear)**2)) + + optimize_result = minimize_scalar(rmse) + if not optimize_result.success: + try: + message = "Optimizer exited unsuccessfully: " \ + + optimize_result.message + except AttributeError: + message = "Optimizer exited unsuccessfully: \ + No message explaining the failure was returned. \ + If you would like to see this message, please \ + update your scipy version (try version 1.8.0 \ + or beyond)." + raise RuntimeError(message) + + else: + alpha = optimize_result.x if round(alpha*10000) == round(previous_alpha*10000): break diff --git a/pvlib/tests/test_clearsky.py b/pvlib/tests/test_clearsky.py index 79502db01c..4e353e997c 100644 --- a/pvlib/tests/test_clearsky.py +++ b/pvlib/tests/test_clearsky.py @@ -1,17 +1,22 @@ from collections import OrderedDict import numpy as np +from numpy import nan import pandas as pd -import pytest import pytz -from numpy import nan -from numpy.testing import assert_allclose from scipy.linalg import hankel -from pvlib import atmosphere, clearsky, irradiance, solarposition +import pytest +from numpy.testing import assert_allclose +from .conftest import assert_frame_equal, assert_series_equal + from pvlib.location import Location +from pvlib import clearsky +from pvlib import solarposition +from pvlib import atmosphere +from pvlib import irradiance -from .conftest import DATA_DIR, assert_frame_equal, assert_series_equal +from .conftest import DATA_DIR def test_ineichen_series(): @@ -672,16 +677,12 @@ def test_detect_clearsky_not_enough_data(detect_clearsky_data): with pytest.raises(ValueError, match='have at least'): clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=60) -@pytest.mark.parametrize("window_length", [5, 10, 15, 20, 25]) -def test_detect_clearsky_optimizer_not_failed( - detect_clearsky_data, - window_length - ): + +def test_detect_clearsky_optimizer_failed(detect_clearsky_data): expected, cs = detect_clearsky_data - clear_samples = clearsky.detect_clearsky( - expected["GHI"], cs["ghi"], window_length=window_length - ) - assert isinstance(clear_samples, pd.Series) + with pytest.raises(RuntimeError, match='Optimizer exited unsuccessfully'): + clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=15) + @pytest.fixture def detect_clearsky_helper_data(): From 171e32e98383cf25b695efb82eb4448fa1d6299e Mon Sep 17 00:00:00 2001 From: Andrew Godbehere Date: Thu, 19 Sep 2024 15:59:23 -0400 Subject: [PATCH 3/5] Reintroduced changes without re-sorting the imports --- docs/sphinx/source/whatsnew/v0.11.1.rst | 5 +++++ pvlib/clearsky.py | 25 +++++-------------------- pvlib/tests/test_clearsky.py | 13 +++++++++---- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/docs/sphinx/source/whatsnew/v0.11.1.rst b/docs/sphinx/source/whatsnew/v0.11.1.rst index 7c8f01743e..781a68f32f 100644 --- a/docs/sphinx/source/whatsnew/v0.11.1.rst +++ b/docs/sphinx/source/whatsnew/v0.11.1.rst @@ -30,6 +30,10 @@ Enhancements * Added function for calculating wind speed at different heights, :py:func:`pvlib.atmosphere.windspeed_powerlaw`. (:issue:`2118`, :pull:`2124`) +* Implemented closed-form solution for alpha in detect_clearsky, obviating + the call to scipy.optimize that was prone to runtime errors and minimizing + computation, :py:func:`pvlib.clearsky.detect_clearsky`. Resolves :issue:`2712`. + Bug fixes ~~~~~~~~~ @@ -74,3 +78,4 @@ Contributors * Marcos R. Escudero (:ghuser:`marc-resc`) * Bernat Nicolau (:ghuser:`BernatNicolau`) * Eduardo Sarquis (:ghuser:`EduardoSarquis`) +* Andrew B Godbehere (:ghuser:`agodbehere`) \ No newline at end of file diff --git a/pvlib/clearsky.py b/pvlib/clearsky.py index 62d83bc1f4..1a32b664ab 100644 --- a/pvlib/clearsky.py +++ b/pvlib/clearsky.py @@ -9,7 +9,6 @@ import numpy as np import pandas as pd -from scipy.optimize import minimize_scalar from scipy.linalg import hankel import h5py @@ -874,25 +873,11 @@ def detect_clearsky(measured, clearsky, times=None, infer_limits=False, clear_meas = meas[clear_samples] clear_clear = clear[clear_samples] - def rmse(alpha): - return np.sqrt(np.mean((clear_meas - alpha*clear_clear)**2)) - - optimize_result = minimize_scalar(rmse) - if not optimize_result.success: - try: - message = "Optimizer exited unsuccessfully: " \ - + optimize_result.message - except AttributeError: - message = "Optimizer exited unsuccessfully: \ - No message explaining the failure was returned. \ - If you would like to see this message, please \ - update your scipy version (try version 1.8.0 \ - or beyond)." - raise RuntimeError(message) - - else: - alpha = optimize_result.x - + # Compute arg min of MSE between model and observations + C = (clear_clear**2).sum() + if not (pd.isna(C) or C == 0): # safety check + # only update alpha if C is strictly positive + alpha = (clear_meas * clear_clear).sum() / C if round(alpha*10000) == round(previous_alpha*10000): break else: diff --git a/pvlib/tests/test_clearsky.py b/pvlib/tests/test_clearsky.py index 4e353e997c..2edb379c15 100644 --- a/pvlib/tests/test_clearsky.py +++ b/pvlib/tests/test_clearsky.py @@ -678,11 +678,16 @@ def test_detect_clearsky_not_enough_data(detect_clearsky_data): clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=60) -def test_detect_clearsky_optimizer_failed(detect_clearsky_data): +@pytest.mark.parametrize("window_length", [5, 10, 15, 20, 25]) +def test_detect_clearsky_optimizer_not_failed( + detect_clearsky_data, + window_length + ): expected, cs = detect_clearsky_data - with pytest.raises(RuntimeError, match='Optimizer exited unsuccessfully'): - clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=15) - + clear_samples = clearsky.detect_clearsky( + expected["GHI"], cs["ghi"], window_length=window_length + ) + assert isinstance(clear_samples, pd.Series) @pytest.fixture def detect_clearsky_helper_data(): From c275a84bbf83e6ce00755f44d96e8d13cc97eb36 Mon Sep 17 00:00:00 2001 From: Andrew Godbehere Date: Fri, 20 Sep 2024 10:08:59 -0400 Subject: [PATCH 4/5] pep8 formatting adjustments --- pvlib/tests/test_clearsky.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pvlib/tests/test_clearsky.py b/pvlib/tests/test_clearsky.py index 2edb379c15..3a014d9f9a 100644 --- a/pvlib/tests/test_clearsky.py +++ b/pvlib/tests/test_clearsky.py @@ -675,20 +675,20 @@ def test_detect_clearsky_missing_index(detect_clearsky_data): def test_detect_clearsky_not_enough_data(detect_clearsky_data): expected, cs = detect_clearsky_data with pytest.raises(ValueError, match='have at least'): - clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=60) + clearsky.detect_clearsky(expected['GHI'], cs['ghi'], window_length=60) @pytest.mark.parametrize("window_length", [5, 10, 15, 20, 25]) def test_detect_clearsky_optimizer_not_failed( - detect_clearsky_data, - window_length - ): + detect_clearsky_data, window_length +): expected, cs = detect_clearsky_data clear_samples = clearsky.detect_clearsky( expected["GHI"], cs["ghi"], window_length=window_length ) assert isinstance(clear_samples, pd.Series) + @pytest.fixture def detect_clearsky_helper_data(): samples_per_window = 3 From bbf5da4bd4eff92c9f65bd06316e34c1dc0dddfd Mon Sep 17 00:00:00 2001 From: Andrew Godbehere Date: Mon, 23 Sep 2024 13:08:47 -0400 Subject: [PATCH 5/5] Update whatsnew as per comments --- docs/sphinx/source/whatsnew/v0.11.1.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sphinx/source/whatsnew/v0.11.1.rst b/docs/sphinx/source/whatsnew/v0.11.1.rst index 781a68f32f..afd1cc7215 100644 --- a/docs/sphinx/source/whatsnew/v0.11.1.rst +++ b/docs/sphinx/source/whatsnew/v0.11.1.rst @@ -30,9 +30,9 @@ Enhancements * Added function for calculating wind speed at different heights, :py:func:`pvlib.atmosphere.windspeed_powerlaw`. (:issue:`2118`, :pull:`2124`) -* Implemented closed-form solution for alpha in detect_clearsky, obviating - the call to scipy.optimize that was prone to runtime errors and minimizing - computation, :py:func:`pvlib.clearsky.detect_clearsky`. Resolves :issue:`2712`. +* Implemented closed-form solution for alpha in :py:func:`pvlib.clearsky.detect_clearsky`, + obviating the call to scipy.optimize that was prone to runtime errors and minimizing + computation. (:issue:`2171`, :issue:`2216`, :pull:`2217`). Bug fixes