Repository: scipy/scipy
Pull request: ENH: `optimize.root`: add warning for invalid inner parameters in `newton_krylov` method
<!-- 
Thanks for contributing a pull request! Please ensure that
your PR satisfies the checklist before submitting:
https://scipy.github.io/devdocs/dev/contributor/development_workflow.html#checklist-before-submitting-a-pr

Also, please name and describe your PR as you would write a
commit message:
https://scipy.github.io/devdocs/dev/contributor/development_workflow.html#writing-the-commit-message.
However, please only include an issue number in the description, not the title,
and please ensure that any code names containing underscores are enclosed in backticks.

Depending on your changes, you can skip CI operations and save time and energy: 
https://scipy.github.io/devdocs/dev/contributor/continuous_integration.html#skipping

Note that we are a team of volunteers; we appreciate your
patience during the review process.

Again, thanks for contributing!
-->

#### Reference issue
<!--Example: Closes gh-WXYZ.-->
Closes #21986 

#### What does this implement/fix?
<!--Please explain your changes.-->
Improves parameter validation in Krylov solver in `optimize.root` by raising warnings for invalid inner parameters but ensuring no warnings for user-provided callable methods.

#### Additional information
<!--Any additional information you think is important.-->


Problem statement:
ENH: optimize.root: warn when inner parameters are ignored with `method='krylov'`
As noted in gh-21296, depending on the "inner" Krylov solver chosen and the `inner_*` options passed, options can just be ignored silently. It would be good to warn the user when an option they pass is invalid and thus ignored.

Hints from issue discussion:
@lucascolley I'd be down to work on this. Do you have a warning message in mind? If so, please let me know.
Something like 

```python
f"The option {inner_*} is not valid for inner solver {solver}. Please see the solver's documentation for a list of valid options."
```
is this issue still up ? can i work in this ?
Hi @Dronten, it looks like this is already being tackled in gh-21993. I have asked the author if they are planning to finish their PR.
Hi @lucascolley , thanks for letting me know , if it is still open to fix i would like to do it 
@Dronten you are welcome to submit a new PR based on gh-21993!
Hi @Dronten, are you planning to submit a PR soon? If not, I’d be happy to work on this. Let me know!

Commit message: Update scipy/optimize/_nonlin.py to resolve linter issues

Co-authored-by: Lucas Colley <lucas.colley8@gmail.com>

Patch to review:
diff --git a/scipy/optimize/_nonlin.py b/scipy/optimize/_nonlin.py
index 7d873765098e..a98391137875 100644
--- a/scipy/optimize/_nonlin.py
+++ b/scipy/optimize/_nonlin.py
@@ -15,6 +15,8 @@
 from scipy._lib._util import copy_if_needed
 from scipy._lib._util import getfullargspec_no_self as _getfullargspec
 from ._linesearch import scalar_search_wolfe1, scalar_search_armijo
+from inspect import signature
+from difflib import get_close_matches
 
 
 __all__ = [
@@ -1491,9 +1493,39 @@ def __init__(self, rdiff=None, method='lgmres', inner_maxiter=20,
             self.method_kw.setdefault('store_outer_Av', False)
             self.method_kw.setdefault('atol', 0)
 
+        # Retrieve the signature of the method to find the valid parameters
+        valid_inner_params = [
+            k for k in signature(self.method).parameters
+            if k not in ('self', 'args', 'kwargs')
+        ]
+
         for key, value in kw.items():
-            if not key.startswith('inner_'):
+            if not key.startswith("inner_"):
                 raise ValueError(f"Unknown parameter {key}")
+            if key[6:] not in valid_inner_params:
+                # Use difflib to find close matches to the invalid key
+                inner_param_suggestions = get_close_matches(key[6:],
+                                                            valid_inner_params,
+                                                            n=1)
+                if inner_param_suggestions:
+                    suggestion_msg = f" Did you mean '"
+                                     f"{inner_param_suggestions[0]}'?"
+                else:
+                    suggestion_msg = ""
+
+                # warn user that the parameter is not valid for the inner method
+                warnings.warn(
+                    f"Option '{key}' is invalid for the inner method: {method}."
+                    " It will be ignored."
+                    "Please check inner method documentation for valid options."
+                    + suggestion_msg,
+                    stacklevel=3,
+                    category=UserWarning,
+                    # using `skip_file_prefixes` would be a good idea
+                    # and should be added once we drop support for Python 3.11
+                )
+                # ignore this parameter and continue
+                continue
             self.method_kw[key[6:]] = value
 
     def _update_diff_step(self):
diff --git a/scipy/optimize/tests/test_nonlin.py b/scipy/optimize/tests/test_nonlin.py
index e5eb094c1590..25b7a6ad8e9d 100644
--- a/scipy/optimize/tests/test_nonlin.py
+++ b/scipy/optimize/tests/test_nonlin.py
@@ -12,6 +12,8 @@
 from numpy.linalg import inv
 import numpy as np
 import scipy
+from scipy.sparse.linalg import minres
+import warnings
 
 from .test_minpack import pressure_network
 
@@ -217,6 +219,41 @@ def wont_converge(x):
         with pytest.raises(scipy.optimize.NoConvergence):
             nonlin.newton_krylov(wont_converge, xin=[0], maxiter=1)
 
+    def test_warnings_invalid_inner_param(self):
+        """
+        Test for ENH #21986, for behavior of `nonlin.newton_krylov`
+        Test the following scenarios:
+        1. Raise warning for invalid inner param
+        2. No warning for valid inner param
+        3. No warning for user-provided callable method
+        """
+
+        # catch all warnings
+        warnings.simplefilter("always")
+
+        # This should raise exactly one warning
+        # (`inner_atol` is not valid for `minres`)
+        with pytest.warns(UserWarning, 
+                          match="Please check inner method documentation"):
+            nonlin.newton_krylov(F, F.xin, method="minres", inner_atol=1e-5)
+
+        # This should not raise a warning (`minres` without `inner_atol`,
+        # but with `inner_maxiter` which is valid)
+        with warnings.catch_warnings(record=True) as w:
+            nonlin.newton_krylov(F, F.xin, method="minres", inner_maxiter=100)
+        assert len(w) == 0
+
+        # Test newton_krylov with a user-provided callable method
+        def user_provided_callable_method_enh_21986(op, rhs, **kwargs):
+            """A dummy user-provided callable method for testing."""
+            # Return a dummy result (mimicking minres)
+            return minres(op, rhs, **kwargs)
+        # This should not raise any warnings
+        with warnings.catch_warnings(record=True) as w:
+            nonlin.newton_krylov(F, F.xin,
+                                method=user_provided_callable_method_enh_21986)
+        assert len(w) == 0
+
 
 class TestSecant:
     """Check that some Jacobian approximations satisfy the secant condition"""


Code review comments:
diff_hunk: @@ -217,6 +219,41 @@ def wont_converge(x):
         with pytest.raises(scipy.optimize.NoConvergence):
             nonlin.newton_krylov(wont_converge, xin=[0], maxiter=1)
 
+    def test_warnings_invalid_inner_param(self):
+        """
+        Test for ENH #21986, for behavior of `nonlin.newton_krylov`
+        Test the following scenarios:
+        1. Raise warning for invalid inner param
+        2. No warning for valid inner param
+        3. No warning for user-provided callable method
+        """
+
+        # catch all warnings
+        warnings.simplefilter("always")
+
+        # This should raise exactly one warning
+        # (`inner_atol` is not valid for `minres`)
+        with pytest.warns(UserWarning, 
+                          match="Please check inner method documentation"):
+            nonlin.newton_krylov(F, F.xin, method="minres", inner_atol=1e-5)
+
+        # This should not raise a warning (`minres` without `inner_atol`,
+        # but with `inner_maxiter` which is valid)
+        with warnings.catch_warnings(record=True) as w:
+            nonlin.newton_krylov(F, F.xin, method="minres", inner_maxiter=100)
+        assert len(w) == 0
+
+        # Test newton_krylov with a user-provided callable method
+        def user_provided_callable_method_enh_21986(op, rhs, **kwargs):
+            """A dummy user-provided callable method for testing."""
+            # Return a dummy result (mimicking minres)
+            return minres(op, rhs, **kwargs)
+        # This should not raise any warnings
+        with warnings.catch_warnings(record=True) as w:
+            nonlin.newton_krylov(F, F.xin,
+                                method=user_provided_callable_method_enh_21986)
+        assert len(w) == 0
path: scipy/optimize/tests/test_nonlin.py
text: @user1:
```suggestion
        nonlin.newton_krylov(F, F.xin, method="minres", inner_maxiter=100)

        # Test newton_krylov with a user-provided callable method
        def user_provided_callable_method_enh_21986(op, rhs, **kwargs):
            """A dummy user-provided callable method for testing."""
            # Return a dummy result (mimicking minres)
            return minres(op, rhs, **kwargs)
        # This should not raise any warnings
        nonlin.newton_krylov(F, F.xin,
                            method=user_provided_callable_method_enh_21986)
```