Sc336/api documentation refresh by sc336 · Pull Request #2012 · GPflow/GPflow

sc336 · 2022-11-01T11:31:26Z

PR type: doc improvement

Related issue(s)/PRs: Contributes to #1823

Summary

A few comments were in places that meant they weren't getting picked up by Sphinx. A few others were unclear enough I had to ask for some help in understanding them; I've updated these to make them more clear.

Fully backwards compatible: yes

PR checklist

st-- · 2022-11-01T14:48:55Z

 class Module(tf.Module):
+    """
+    Modules recursively compose other Modules and parameters to create models.
+    All GPFlow models, kernels, mean functions etc. are Modules.


Suggested change

All GPFlow models, kernels, mean functions etc. are Modules.

All GPflow models, kernels, mean functions etc. are Modules.

st-- · 2022-11-01T14:49:51Z

+    See also `this <https://www.tensorflow.org/api_docs/python/tf/Module>`_ documentation for the base class in
+    Tensorflow which goes into more detail as to why we use Module objects to compose things.


Suggested change

See also `this <https://www.tensorflow.org/api_docs/python/tf/Module>`_ documentation for the base class in

Tensorflow which goes into more detail as to why we use Module objects to compose things.

See also `TensorFlow's documentation <https://www.tensorflow.org/api_docs/python/tf/Module>`_ for the base class

which goes into more detail as to why we use Module objects to compose things.

st-- · 2022-11-01T14:53:23Z


 class Module(tf.Module):
+    """
+    Modules recursively compose other Modules and parameters to create models.


Suggested change

Modules recursively compose other Modules and parameters to create models.

Modules recursively compose other Modules and parameters to create models.

Compared to the `tf.Module` base class, `gpflow.Module` includes additional support for handling `gpflow.Parameter` attributes, see :py:attr:`~parameters` and :py:attr:`~trainable_parameters`.

It also adds pretty-printing within IPython and Jupyter notebooks.

to explain why there's a separate class

Nice, thanks

(as an aside, it would be nice if the pretty printing were accessible via a public method!)

st-- · 2022-11-01T14:55:52Z

+    is provided, these two values will be the same.  It is often challenging to operate with
+    unconstrained parameters. For example, a variance cannot be negative, therefore we need a
+    positive constraint and it is natural to use constrained values.  A prior can be imposed


While we're looking at this, would it be worth clarifying that for us (humans), the constrained space is "natural" and "easy", whereas for the optimizer, constraints are hard and the unconstrained space is natural/easy, which is why we want to present it to the optimizer in unconstrained form?

Sounds like a good idea.

st-- · 2022-11-01T14:57:03Z

    independently for each row of X.

+    This class is not only used for mean functions.  For example, the variance of a
+    heteroskedastic model is specified using a Function.


This is only true for some heteroskedastic models, when the variance is given in a parametric form. You can also build heteroskedastic models where the variance is modeled with another GP. Then it doesn't use this class.

Suggested change

heteroskedastic model is specified using a Function.

heteroskedastic model can be specified using a `Function`.

and add a link to the corresponding notebook?

codecov · 2022-11-01T15:47:27Z

Codecov Report

Base: 98.09% // Head: 98.09% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (29c2e0d) compared to base (f66a708).
Patch has no changes to coverable lines.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2012      +/-   ##
===========================================
- Coverage    98.09%   98.09%   -0.01%     
===========================================
  Files           93       93              
  Lines         5262     5261       -1     
===========================================
- Hits          5162     5161       -1     
  Misses         100      100

Impacted Files	Coverage Δ
gpflow/functions.py	`96.18% <ø> (ø)`
gpflow/kernels/__init__.py	`100.00% <ø> (ø)`
gpflow/kernels/base.py	`93.05% <ø> (-0.05%)`	⬇️
gpflow/models/model.py	`98.64% <ø> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

uri-granta

LGTM!

uri-granta · 2022-11-02T08:11:02Z

 class Parameter(tfp.util.TransformedVariable):
+    """A parameter retains both constrained and unconstrained representations. If no transform
+    is provided, these two values will be the same.  It is often challenging for humans to operate with
+    unconstrained parameters, although this is typically easier for the optimiser. For example, a variance cannot be negative, therefore we need a


Isn't this giving "line too long" linting errors? Either way, should probably rejustify.

uri-granta · 2022-11-02T08:15:46Z

        type=str,
        nargs="*",
-        help="Only process the notebooks with this base/stem name. Useful when debugging.",
+        help="Only process the notebooks with this base/stem name; this is typically much faster and is useful when debugging.  For example, to process notebooks/tailor/external-mean-function.pct.py, use --limit_notebooks external-mean-function\n",


Suggested change

help="Only process the notebooks with this base/stem name; this is typically much faster and is useful when debugging. For example, to process notebooks/tailor/external-mean-function.pct.py, use --limit_notebooks external-mean-function\n",

help="Only process the notebooks with this base/stem name; this is much faster and is useful when debugging. For example, to process notebooks/tailor/external-mean-function.pct.py, use --limit_notebooks external-mean-function\n",

uri-granta · 2022-11-02T08:18:06Z


 class Module(tf.Module):
+    """
+    Modules recursively compose other Modules and parameters to create models.


(as an aside, it would be nice if the pretty printing were accessible via a public method!)

uri-granta · 2022-11-02T08:19:12Z

    ) -> tf.Tensor:
        """
-        Compute the log density of the data at the new data points.
+        Compute the log of the probability density of the data at the new data points.


Genuine question: is log-density not a standard term?

I think it's standard enough, but it doesn't hurt to be a bit verbose in the docstring. The function is already called log_density..

st-- · 2022-11-02T12:48:10Z

@uri-granta

(as an aside, it would be nice if the pretty printing were accessible via a public method!)

gpflow.utilities.print_summary() ?

sc336 requested a review from uri-granta November 1, 2022 11:40

sc336 added 5 commits November 1, 2022 11:50

Various docstring improvements

75f8e27

Various docstring improvements

a647aa5

Added and improved some comments

a6ffdd2

Formatting changes

dc9847c

Added myself as new contributor

db3e9b6

sc336 force-pushed the sc336/api_documentation_refresh branch from 304a1f6 to db3e9b6 Compare November 1, 2022 11:50

st-- reviewed Nov 1, 2022

View reviewed changes

More commenting

bde6cb7

st-- reviewed Nov 1, 2022

View reviewed changes

Edits made based on review from st--

29c2e0d

uri-granta approved these changes Nov 2, 2022

View reviewed changes

sc336 merged commit 26a27a4 into develop Nov 2, 2022

sc336 deleted the sc336/api_documentation_refresh branch November 7, 2022 14:04

	All GPFlow models, kernels, mean functions etc. are Modules.
	All GPflow models, kernels, mean functions etc. are Modules.

		See also `this <https://www.tensorflow.org/api_docs/python/tf/Module>`_ documentation for the base class in
		Tensorflow which goes into more detail as to why we use Module objects to compose things.

	heteroskedastic model is specified using a Function.
	heteroskedastic model can be specified using a `Function`.

	help="Only process the notebooks with this base/stem name; this is typically much faster and is useful when debugging. For example, to process notebooks/tailor/external-mean-function.pct.py, use --limit_notebooks external-mean-function\n",
	help="Only process the notebooks with this base/stem name; this is much faster and is useful when debugging. For example, to process notebooks/tailor/external-mean-function.pct.py, use --limit_notebooks external-mean-function\n",

Conversation

sc336 commented Nov 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

PR checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Nov 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

uri-granta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

st-- commented Nov 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sc336 commented Nov 1, 2022 •

edited

Loading

codecov Bot commented Nov 1, 2022 •

edited

Loading