Skip to content

[WIP] Correct kernel docstrings to explain evaluation of gradient#8412

Closed
dalmia wants to merge 1 commit intoscikit-learn:masterfrom
dalmia:8393
Closed

[WIP] Correct kernel docstrings to explain evaluation of gradient#8412
dalmia wants to merge 1 commit intoscikit-learn:masterfrom
dalmia:8393

Conversation

@dalmia
Copy link
Copy Markdown
Contributor

@dalmia dalmia commented Feb 20, 2017

Reference Issue

Fixes #8393

What does this implement/fix? Explain your changes.

This corrects the docstrings of the GaussianProcess kernels to indicate that when eval_gradient is True, the matrix returned is the gradient of the log-transformed hyperparameters of the kernel and not that of the hyperparameters. Also, I've added comments in sections of the code indicating how the gradient has been calculated.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 20, 2017

Codecov Report

Merging #8412 into master will decrease coverage by 1.44%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8412      +/-   ##
==========================================
- Coverage   96.19%   94.75%   -1.45%     
==========================================
  Files         348      342       -6     
  Lines       64645    60816    -3829     
==========================================
- Hits        62187    57624    -4563     
- Misses       2458     3192     +734
Impacted Files Coverage Δ
sklearn/gaussian_process/kernels.py 91.54% <ø> (ø) ⬆️
sklearn/feature_extraction/tests/test_image.py 4.78% <0%> (-95.22%) ⬇️
sklearn/feature_extraction/image.py 58.16% <0%> (-41.84%) ⬇️
sklearn/utils/arpack.py 42.5% <0%> (-32.5%) ⬇️
sklearn/utils/random.py 59.29% <0%> (-32.2%) ⬇️
sklearn/datasets/tests/test_kddcup99.py 29.16% <0%> (-10.84%) ⬇️
sklearn/manifold/mds.py 84.46% <0%> (-9.71%) ⬇️
sklearn/utils/tests/test_estimator_checks.py 88.23% <0%> (-8.96%) ⬇️
sklearn/linear_model/tests/test_bayes.py 83.01% <0%> (-8.16%) ⬇️
sklearn/utils/tests/test_utils.py 92.94% <0%> (-7.06%) ⬇️
... and 260 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e29334...2ea9dce. Read the comment docs.

Copy link
Copy Markdown
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is too much information in the Parameters sections. We should be making this change in the Returns sections only

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented Aug 16, 2020

Closed in #18115

@cmarmo cmarmo closed this Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docstring of Gaussian processes kernel wrong about the calculation of gradient

3 participants