From bc24486df53f35c15009a432a7c8aa6857c8201b Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Mon, 18 Aug 2025 16:47:10 -0400 Subject: [PATCH 1/8] raise `ValueError` if the the number of regression points is less than the number of parameters in `_residual` --- news/enough-regression-points.rst | 23 +++++++++++++++++++++++ src/diffpy/morph/refine.py | 11 ++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 news/enough-regression-points.rst diff --git a/news/enough-regression-points.rst b/news/enough-regression-points.rst new file mode 100644 index 0000000..8ff9e7c --- /dev/null +++ b/news/enough-regression-points.rst @@ -0,0 +1,23 @@ +**Added:** + +* Raise ``ValueError`` if the number of shared grid points between morphed and target functions is less than the number of parameters. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/src/diffpy/morph/refine.py b/src/diffpy/morph/refine.py index 688e06e..36cd244 100644 --- a/src/diffpy/morph/refine.py +++ b/src/diffpy/morph/refine.py @@ -83,7 +83,13 @@ def _residual(self, pvals): self.x_morph, self.y_morph, self.x_target, self.y_target ) rvec = _y_target - _y_morph - + if len(rvec) < len(pvals): + raise ValueError( + "Not enough shared grid points between morphed function " + "and target function to fit the chosen parameters. " + "Please adjust the functions or " + "reduce the number of parameters." + ) # If first time computing residual if self.res_length is None: self.res_length = len(rvec) @@ -145,6 +151,9 @@ def refine(self, *args, **kw): ------ ValueError Exception raised if a minimum cannot be found. + ValueError + If the number of shared grid points between morphed function and + target function is smaller than the number of parameters. """ self.pars = args or self.chain.config.keys() From f5375120bd701a5a9c9c65b08bc9129840db18de Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Wed, 20 Aug 2025 18:48:12 -0400 Subject: [PATCH 2/8] test: add test for not enough shared grid points the bad case. --- src/diffpy/morph/refine.py | 8 ++++-- tests/test_refine.py | 58 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/diffpy/morph/refine.py b/src/diffpy/morph/refine.py index 36cd244..aa37382 100644 --- a/src/diffpy/morph/refine.py +++ b/src/diffpy/morph/refine.py @@ -85,9 +85,13 @@ def _residual(self, pvals): rvec = _y_target - _y_morph if len(rvec) < len(pvals): raise ValueError( + f"\nNumber of shared grid points: {len(rvec)}\n" + f"Number of parameters: {len(pvals)}\n" "Not enough shared grid points between morphed function " - "and target function to fit the chosen parameters. " - "Please adjust the functions or " + "between morphed function and target function to fit " + "the chosen parameters.\n" + "Please make sure the overlapping domain between the morphed " + "function and the target function is sufficiently large, or " "reduce the number of parameters." ) # If first time computing residual diff --git a/tests/test_refine.py b/tests/test_refine.py index 4cf1bc7..01b14cd 100644 --- a/tests/test_refine.py +++ b/tests/test_refine.py @@ -181,6 +181,64 @@ def stretch(x, y, stretch): assert res < err + def test_refine_grid_bad(self, user_filesystem): + grid = numpy.arange(2) + func = numpy.sin(grid) + grid1, func1, grid2, func2 = grid, func, grid, func + config = { + "stretch": 0.005, + "scale": 1.0, + "smear": 0, + } + chain = MorphChain(config) + refiner = Refiner(chain, grid1, func1, grid2, func2) + refpars = ["stretch", "scale", "smear"] + expected_error_message = ( + "\nNumber of shared grid points: 2\n" + "Number of parameters: 3\n" + "Not enough shared grid points between morphed function " + "between morphed function and target function to fit " + "the chosen parameters.\n" + "Please make sure the overlapping domain between the morphed " + "function and the target function is sufficiently large, or " + "reduce the number of parameters." + ) + with pytest.raises( + ValueError, + ) as error: + refiner.refine(*refpars) + actual_error_message = str(error.value) + assert actual_error_message == expected_error_message + + # call from command line + import subprocess + + data_dir_path = user_filesystem / "cwd_dir" + morph_file = data_dir_path / "morph_data" + morph_data_text = [ + str(grid1[i]) + " " + str(func1[i]) for i in range(len(grid1)) + ] + morph_data_text = "\n".join(morph_data_text) + morph_file.write_text(morph_data_text) + target_file = data_dir_path / "target_data" + target_data_text = [ + str(grid2[i]) + " " + str(func2[i]) for i in range(len(grid2)) + ] + target_data_text = "\n".join(target_data_text) + target_file.write_text(target_data_text) + run_cmd = ["diffpy.morph"] + for key, value in config.items(): + run_cmd.append(f"--{key}") + run_cmd.append(f"{value}") + run_cmd.extend([str(morph_file), str(target_file)]) + run_cmd.append("-n") + result = subprocess.run(run_cmd, capture_output=True, text=True) + expected_error_message = ( + "diffpy.morph: error: " + expected_error_message + ) + actual_error_message = result.stderr.strip() + assert actual_error_message == expected_error_message + # End of class TestRefine From 9832900b2a266e6d75aeeab33a0be49e53c1e8b7 Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Wed, 20 Aug 2025 19:01:57 -0400 Subject: [PATCH 3/8] chore: fix typo --- src/diffpy/morph/refine.py | 4 ++-- tests/test_refine.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/diffpy/morph/refine.py b/src/diffpy/morph/refine.py index aa37382..94a0a0c 100644 --- a/src/diffpy/morph/refine.py +++ b/src/diffpy/morph/refine.py @@ -87,8 +87,8 @@ def _residual(self, pvals): raise ValueError( f"\nNumber of shared grid points: {len(rvec)}\n" f"Number of parameters: {len(pvals)}\n" - "Not enough shared grid points between morphed function " - "between morphed function and target function to fit " + "Not enough shared grid points " + "between the morphed function and the target function to fit " "the chosen parameters.\n" "Please make sure the overlapping domain between the morphed " "function and the target function is sufficiently large, or " diff --git a/tests/test_refine.py b/tests/test_refine.py index 01b14cd..56e8ed0 100644 --- a/tests/test_refine.py +++ b/tests/test_refine.py @@ -196,8 +196,8 @@ def test_refine_grid_bad(self, user_filesystem): expected_error_message = ( "\nNumber of shared grid points: 2\n" "Number of parameters: 3\n" - "Not enough shared grid points between morphed function " - "between morphed function and target function to fit " + "Not enough shared grid points " + "between the morphed function and the target function to fit " "the chosen parameters.\n" "Please make sure the overlapping domain between the morphed " "function and the target function is sufficiently large, or " From 71f10e585617be3b09aa79023756ed338b558a41 Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Thu, 21 Aug 2025 18:16:54 -0400 Subject: [PATCH 4/8] chore: more concise error message --- src/diffpy/morph/refine.py | 14 ++++++-------- tests/test_refine.py | 14 ++++++-------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/diffpy/morph/refine.py b/src/diffpy/morph/refine.py index 94a0a0c..8f1390d 100644 --- a/src/diffpy/morph/refine.py +++ b/src/diffpy/morph/refine.py @@ -85,14 +85,12 @@ def _residual(self, pvals): rvec = _y_target - _y_morph if len(rvec) < len(pvals): raise ValueError( - f"\nNumber of shared grid points: {len(rvec)}\n" - f"Number of parameters: {len(pvals)}\n" - "Not enough shared grid points " - "between the morphed function and the target function to fit " - "the chosen parameters.\n" - "Please make sure the overlapping domain between the morphed " - "function and the target function is sufficiently large, or " - "reduce the number of parameters." + f"\nNumber of parameters (currently {len(pvals)}) cannot " + "exceed the number of shared grid points " + f"(currently {len(rvec)}). " + "Please reduce the number of morphing parameters or " + "provide new morphing and target functions with more " + "shared grid points." ) # If first time computing residual if self.res_length is None: diff --git a/tests/test_refine.py b/tests/test_refine.py index 56e8ed0..f3cecdc 100644 --- a/tests/test_refine.py +++ b/tests/test_refine.py @@ -194,14 +194,12 @@ def test_refine_grid_bad(self, user_filesystem): refiner = Refiner(chain, grid1, func1, grid2, func2) refpars = ["stretch", "scale", "smear"] expected_error_message = ( - "\nNumber of shared grid points: 2\n" - "Number of parameters: 3\n" - "Not enough shared grid points " - "between the morphed function and the target function to fit " - "the chosen parameters.\n" - "Please make sure the overlapping domain between the morphed " - "function and the target function is sufficiently large, or " - "reduce the number of parameters." + f"\nNumber of parameters (currently 3) cannot " + "exceed the number of shared grid points " + f"(currently 2). " + "Please reduce the number of morphing parameters or " + "provide new morphing and target functions with more " + "shared grid points." ) with pytest.raises( ValueError, From 935dd8ed08a3a9694ea7a21f97f98cd24e4c2cdb Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Thu, 21 Aug 2025 18:18:12 -0400 Subject: [PATCH 5/8] chore: fix pre-commit --- tests/test_refine.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_refine.py b/tests/test_refine.py index f3cecdc..bf55535 100644 --- a/tests/test_refine.py +++ b/tests/test_refine.py @@ -194,9 +194,9 @@ def test_refine_grid_bad(self, user_filesystem): refiner = Refiner(chain, grid1, func1, grid2, func2) refpars = ["stretch", "scale", "smear"] expected_error_message = ( - f"\nNumber of parameters (currently 3) cannot " + "\nNumber of parameters (currently 3) cannot " "exceed the number of shared grid points " - f"(currently 2). " + "(currently 2). " "Please reduce the number of morphing parameters or " "provide new morphing and target functions with more " "shared grid points." From 91fa347f8e783bdb3be5423eabdfd7b234ff63ff Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Thu, 21 Aug 2025 23:22:39 -0400 Subject: [PATCH 6/8] fix: use a custom error to stop `leatsq` --- src/diffpy/morph/log.py | 9 +++++++++ src/diffpy/morph/morphapp.py | 3 ++- src/diffpy/morph/refine.py | 7 ++++--- tests/test_refine.py | 7 ++++--- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/diffpy/morph/log.py b/src/diffpy/morph/log.py index c191956..6c47206 100644 --- a/src/diffpy/morph/log.py +++ b/src/diffpy/morph/log.py @@ -50,4 +50,13 @@ def set_verbosity(vb): return +class MorphOptimizationError(Exception): + "Custom error for morph optimization process." + + def __init__(self, value): + super().__init__(value) + + pass + + # End of file diff --git a/src/diffpy/morph/morphapp.py b/src/diffpy/morph/morphapp.py index 9981e3a..0defff3 100755 --- a/src/diffpy/morph/morphapp.py +++ b/src/diffpy/morph/morphapp.py @@ -27,6 +27,7 @@ import diffpy.morph.refine as refine import diffpy.morph.tools as tools from diffpy.morph import __save_morph_as__ +from diffpy.morph.log import MorphOptimizationError from diffpy.morph.version import __version__ @@ -680,7 +681,7 @@ def single_morph( refiner.refine(*rptemp) # Adjust all parameters refiner.refine(*refpars) - except ValueError as e: + except (ValueError, MorphOptimizationError) as e: parser.custom_error(str(e)) # Smear is not being refined, but baselineslope needs to refined to apply # smear diff --git a/src/diffpy/morph/refine.py b/src/diffpy/morph/refine.py index 8f1390d..9ebf67b 100644 --- a/src/diffpy/morph/refine.py +++ b/src/diffpy/morph/refine.py @@ -12,13 +12,14 @@ # See LICENSE.txt for license information. # ############################################################################## -"""refine -- Refine a morph or morph chain -""" +"""refine -- Refine a morph or morph chain""" from numpy import array, concatenate, dot, exp, ones_like from scipy.optimize import leastsq from scipy.stats import pearsonr +from diffpy.morph.log import MorphOptimizationError + # Map of scipy minimizer names to the method that uses them @@ -84,7 +85,7 @@ def _residual(self, pvals): ) rvec = _y_target - _y_morph if len(rvec) < len(pvals): - raise ValueError( + raise MorphOptimizationError( f"\nNumber of parameters (currently {len(pvals)}) cannot " "exceed the number of shared grid points " f"(currently {len(rvec)}). " diff --git a/tests/test_refine.py b/tests/test_refine.py index bf55535..51927ef 100644 --- a/tests/test_refine.py +++ b/tests/test_refine.py @@ -6,6 +6,7 @@ import numpy import pytest +from diffpy.morph.log import MorphOptimizationError from diffpy.morph.morph_helpers.transformpdftordf import TransformXtalPDFtoRDF from diffpy.morph.morph_helpers.transformrdftopdf import TransformXtalRDFtoPDF from diffpy.morph.morphs.morphchain import MorphChain @@ -202,11 +203,11 @@ def test_refine_grid_bad(self, user_filesystem): "shared grid points." ) with pytest.raises( - ValueError, + MorphOptimizationError, ) as error: refiner.refine(*refpars) - actual_error_message = str(error.value) - assert actual_error_message == expected_error_message + actual_error_message = str(error.value) + assert actual_error_message == expected_error_message # call from command line import subprocess From 5b0f30ba06717b5f5dd3b681c535f8fffdbfe772 Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Thu, 21 Aug 2025 23:34:39 -0400 Subject: [PATCH 7/8] Revert "fix: use a custom error to stop `leatsq`" This reverts commit 91fa347f8e783bdb3be5423eabdfd7b234ff63ff. --- src/diffpy/morph/log.py | 9 --------- src/diffpy/morph/morphapp.py | 3 +-- src/diffpy/morph/refine.py | 7 +++---- tests/test_refine.py | 7 +++---- 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/diffpy/morph/log.py b/src/diffpy/morph/log.py index 6c47206..c191956 100644 --- a/src/diffpy/morph/log.py +++ b/src/diffpy/morph/log.py @@ -50,13 +50,4 @@ def set_verbosity(vb): return -class MorphOptimizationError(Exception): - "Custom error for morph optimization process." - - def __init__(self, value): - super().__init__(value) - - pass - - # End of file diff --git a/src/diffpy/morph/morphapp.py b/src/diffpy/morph/morphapp.py index 0defff3..9981e3a 100755 --- a/src/diffpy/morph/morphapp.py +++ b/src/diffpy/morph/morphapp.py @@ -27,7 +27,6 @@ import diffpy.morph.refine as refine import diffpy.morph.tools as tools from diffpy.morph import __save_morph_as__ -from diffpy.morph.log import MorphOptimizationError from diffpy.morph.version import __version__ @@ -681,7 +680,7 @@ def single_morph( refiner.refine(*rptemp) # Adjust all parameters refiner.refine(*refpars) - except (ValueError, MorphOptimizationError) as e: + except ValueError as e: parser.custom_error(str(e)) # Smear is not being refined, but baselineslope needs to refined to apply # smear diff --git a/src/diffpy/morph/refine.py b/src/diffpy/morph/refine.py index 9ebf67b..8f1390d 100644 --- a/src/diffpy/morph/refine.py +++ b/src/diffpy/morph/refine.py @@ -12,14 +12,13 @@ # See LICENSE.txt for license information. # ############################################################################## -"""refine -- Refine a morph or morph chain""" +"""refine -- Refine a morph or morph chain +""" from numpy import array, concatenate, dot, exp, ones_like from scipy.optimize import leastsq from scipy.stats import pearsonr -from diffpy.morph.log import MorphOptimizationError - # Map of scipy minimizer names to the method that uses them @@ -85,7 +84,7 @@ def _residual(self, pvals): ) rvec = _y_target - _y_morph if len(rvec) < len(pvals): - raise MorphOptimizationError( + raise ValueError( f"\nNumber of parameters (currently {len(pvals)}) cannot " "exceed the number of shared grid points " f"(currently {len(rvec)}). " diff --git a/tests/test_refine.py b/tests/test_refine.py index 51927ef..bf55535 100644 --- a/tests/test_refine.py +++ b/tests/test_refine.py @@ -6,7 +6,6 @@ import numpy import pytest -from diffpy.morph.log import MorphOptimizationError from diffpy.morph.morph_helpers.transformpdftordf import TransformXtalPDFtoRDF from diffpy.morph.morph_helpers.transformrdftopdf import TransformXtalRDFtoPDF from diffpy.morph.morphs.morphchain import MorphChain @@ -203,11 +202,11 @@ def test_refine_grid_bad(self, user_filesystem): "shared grid points." ) with pytest.raises( - MorphOptimizationError, + ValueError, ) as error: refiner.refine(*refpars) - actual_error_message = str(error.value) - assert actual_error_message == expected_error_message + actual_error_message = str(error.value) + assert actual_error_message == expected_error_message # call from command line import subprocess From daf51c3a621156938ea6646d3d1384dbda47a9bf Mon Sep 17 00:00:00 2001 From: Yuchen Ethan Xiao Date: Thu, 21 Aug 2025 23:36:02 -0400 Subject: [PATCH 8/8] chore: fix the code indentation. --- tests/test_refine.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_refine.py b/tests/test_refine.py index bf55535..76263bf 100644 --- a/tests/test_refine.py +++ b/tests/test_refine.py @@ -205,8 +205,8 @@ def test_refine_grid_bad(self, user_filesystem): ValueError, ) as error: refiner.refine(*refpars) - actual_error_message = str(error.value) - assert actual_error_message == expected_error_message + actual_error_message = str(error.value) + assert actual_error_message == expected_error_message # call from command line import subprocess