Add stricter gradient check for log marginal likelihood in Gaussian Processes #31543

snath-xoc · 2025-06-13T14:17:18Z

First step in fixing the error in the log-marginal likelihood gradient calculations in Gaussian Processes as noticed in #31366. Also related to #31289.

What does this implement/fix? Explain your changes.

Implements a stricter test for test_lml_gradient by replacing the manual gradient calculation usingscip.optimize.approx_fprime with scipy.differentiate.derivative and calculating the gradient over several different length_scales

@conradstevens and @lorentzenchr any suggestions are welcome (cc @ogrisel and @antoinebaker)

TO DO (perhaps within this PR or separately):

Implement chain rule calculation for theta under GaussianProcessRegressor
Implement chain rule calculation for the Max Absolute Posteriori (b) in _BinaryGaussianProcessClassifierLaplace

github-actions · 2025-06-13T14:18:11Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: da433c0. Link to the linter CI: here}

snath-xoc · 2025-06-13T14:18:34Z

Somehow some commits from other branches got mixed up in this PR hence the weird number of commit messages (the files should be restored now)

conradstevens · 2025-06-14T01:00:09Z

Are we going to go with scipy.differentiate as suggested by @lorentzenchr?

snath-xoc · 2025-06-19T13:58:15Z

@conradstevens yes, your initial manual_grad implementation is similar to approx_fprime and derivative seems to yield similar results. I think they could be used interchangably, derivative is roughly based off taylor series approximation and seems to be more correct. The main addition is that we check it over many different length-scales which is different from the previous implementation.

lorentzenchr · 2025-07-16T05:36:10Z

Could you now mark the failing tests with pytest‘s xfail?
Then another PR can fix the gradient.

ogrisel · 2025-07-28T09:09:16Z

Let me merge main into this branch to get up-to-date CI reports.

ogrisel

Thanks for the PR, here is a first pass of reviews.

ogrisel · 2025-07-28T09:17:19Z

sklearn/gaussian_process/tests/test_gpc.py

@@ -105,17 +105,37 @@ def test_converged_to_local_maximum(kernel):
    )


-@pytest.mark.parametrize("kernel", kernels)
+@pytest.mark.parametrize("kernel", non_fixed_kernels[:-1])


Could you please include the product kernel in this list?

The code could be updated to find the name of the relevant kernel parameter using:

length_scale_param_name = next( name for name in k.get_params() if name.endswith("length_scale") )

and then the for loop can be updated to use kernel.set_params(**{length_scale_param_name: length_scale}) instead of kernel.length_scale = length_scale.

ogrisel · 2025-07-28T09:23:38Z

sklearn/gaussian_process/tests/test_gpc.py

+            1
+        ][0]
+
+    lml_gradient_manual = derivative(


I prefer to keep using the lml_gradient_approx name for this variable, as scipy.differentiate.derivative is doing an approximate (numerical) computation rather than a manually implemented (symbolically derived) computation.

ogrisel · 2025-07-28T09:33:26Z

sklearn/gaussian_process/tests/test_gpc.py

-    lml_gradient_approx = approx_fprime(
-        kernel.theta, lambda theta: gpc.log_marginal_likelihood(theta, False), 1e-10
-    )
+    length_scales = np.linspace(1, 25, 1_000)


I would rather use np.logspace(-3, 3, 100):

reducing the number of steps should make the test run faster;

using a logspace on this range, typically used to configure such parameter bounds, should still check for most interesting values of that parameter.

ogrisel · 2025-07-28T09:58:09Z

sklearn/gaussian_process/tests/test_gpr.py

@@ -140,17 +140,36 @@ def test_solution_inside_bounds(kernel):
    assert_array_less(gpr.kernel_.theta, bounds[:, 1] + tiny)


-@pytest.mark.parametrize("kernel", kernels)
+@pytest.mark.parametrize("kernel", non_fixed_kernels[:2])
 def test_lml_gradient(kernel):


Suggested change

def test_lml_gradient(kernel):

def test_lml_gradient(kernel):

# Clone the kernel object prior to mutating it to avoid any side effects between

# GP tests:

kernel = clone(kernel)

ogrisel · 2025-07-28T09:58:56Z

sklearn/gaussian_process/tests/test_gpc.py

@@ -105,17 +105,37 @@ def test_converged_to_local_maximum(kernel):
    )


-@pytest.mark.parametrize("kernel", kernels)
+@pytest.mark.parametrize("kernel", non_fixed_kernels[:-1])
 def test_lml_gradient(kernel):


I think we should always clone the kernel argument passed to test functions: otherwise, mutation of this object can cause other unrelated tests to fail (as is the case in this PR).

Suggested change

def test_lml_gradient(kernel):

def test_lml_gradient(kernel):

# Clone the kernel object prior to mutating it to avoid any side effects between

# GP tests:

kernel = clone(kernel)

Note that adding from sklearn.base import clone is also needed.

Note: this change might also be needed to get a thread-safe test suite (as being investigated in the draft #30041).

ogrisel · 2025-07-28T10:00:23Z

sklearn/gaussian_process/tests/test_gpc.py

+    length_scales = np.linspace(1, 25, 1_000)
+
+    def evaluate_grad_at_length_scales(length_scales):
+        result = np.zeros_like(length_scales)


The shape of the result array seems invalid when len(kernel.theta) != 1.

Instead, you could store the results in a Python list and then stack the results.

ogrisel · 2025-07-28T10:01:27Z

sklearn/gaussian_process/tests/test_gpr.py

@@ -140,17 +140,36 @@ def test_solution_inside_bounds(kernel):
    assert_array_less(gpr.kernel_.theta, bounds[:, 1] + tiny)


-@pytest.mark.parametrize("kernel", kernels)
+@pytest.mark.parametrize("kernel", non_fixed_kernels[:2])


Please test against all non_fixed_kernels or add a comment to explain why some kernels are left out of this test (if there is a good reason for that).

snath-xoc added 12 commits April 25, 2025 16:38

lazy test added

82706dd

Merge github.com:snath-xoc/scikit-learn

1908831

kerge branch 'main' of github.com:snath-xoc/scikit-learn

ffcf4d4

Merge branch 'main' of github.com:snath-xoc/scikit-learn

2184bff

modify tests

1fa4984

modify lml gradient tests

e11bc4e

modify lml grad tests

92f04fe

lazy test added

06d6139

Merge branch 'main' into add_gradient_check_gpr

33db8c2

merge main

68bf517

Merge branch 'main' into add_gradient_check_gpr

5da4237

restore sag

bbbc657

github-actions bot added the module:gaussian_process label Jun 13, 2025

Merge branch 'main' into add_gradient_check_gpr

734abfd

Merge branch 'main' into add_gradient_check_gpr

da433c0

ogrisel reviewed Jul 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add stricter gradient check for log marginal likelihood in Gaussian Processes #31543

Add stricter gradient check for log marginal likelihood in Gaussian Processes #31543

snath-xoc commented Jun 13, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jun 13, 2025 •

edited

Loading

Uh oh!

snath-xoc commented Jun 13, 2025

Uh oh!

conradstevens commented Jun 14, 2025

Uh oh!

snath-xoc commented Jun 19, 2025 •

edited

Loading

Uh oh!

lorentzenchr commented Jul 16, 2025

Uh oh!

ogrisel commented Jul 28, 2025

Uh oh!

ogrisel left a comment

Uh oh!

ogrisel Jul 28, 2025

Uh oh!

ogrisel Jul 28, 2025

Uh oh!

ogrisel Jul 28, 2025

Uh oh!

ogrisel Jul 28, 2025

Uh oh!

ogrisel Jul 28, 2025

Uh oh!

ogrisel Jul 28, 2025

Uh oh!

ogrisel Jul 28, 2025

Uh oh!

Uh oh!

-def test_lml_gradient(kernel):
+def test_lml_gradient(kernel):
+    # Clone the kernel object prior to mutating it to avoid any side effects between
+    # GP tests:
+    kernel = clone(kernel)

Uh oh!

Add stricter gradient check for log marginal likelihood in Gaussian Processes #31543

Are you sure you want to change the base?

Add stricter gradient check for log marginal likelihood in Gaussian Processes #31543

Conversation

snath-xoc commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this implement/fix? Explain your changes.

Uh oh!

github-actions bot commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

snath-xoc commented Jun 13, 2025

Uh oh!

conradstevens commented Jun 14, 2025

Uh oh!

snath-xoc commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lorentzenchr commented Jul 16, 2025

Uh oh!

ogrisel commented Jul 28, 2025

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

ogrisel Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

snath-xoc commented Jun 13, 2025 •

edited

Loading

github-actions bot commented Jun 13, 2025 •

edited

Loading

snath-xoc commented Jun 19, 2025 •

edited

Loading