Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes bugs in optimizer docstrings #1053

Merged
merged 1 commit into from
Jan 30, 2021
Merged

Fixes bugs in optimizer docstrings #1053

merged 1 commit into from
Jan 30, 2021

Conversation

josh146
Copy link
Member

@josh146 josh146 commented Jan 30, 2021

Context:

  • Several of the optimizer docstrings had rendering bugs and outdated terminology

Description of the Change:

  • Removes rendering bugs in the Returns: statement by removing indentation in continuation lines. While the Args: section requires continuation lines be indented since there might be multiple arguments, there is only ever a single return, and so the return is never indented.

  • In Allowing more flexible cost functions for optimizers #959, most of the optimizers were updated to remove restrictions on cost function signatures. As a result, the x argument was changed to *args to be more general. Some stray x mentions in the docstrings were removed.

Benefits: Docstrings render correctly.

Possible Drawbacks:

I noticed while working on this that the QNGOptimizer is the one optimizer that continues to only accept cost functions with a single argument. This was intentional in #959, as at the time the metric_tensor method did not allow multiple QNode arguments.

However, the new qml.metric_tensor function that was added in #1014 removes this restriction, so we are now unblocked from also updating the QNGOptimizer.

Related GitHub Issues: n/a

@josh146 josh146 added the documentation 📘 Documentation changes and updates label Jan 30, 2021
@josh146 josh146 requested a review from co9olguy January 30, 2021 14:16
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

Comment on lines 65 to -67
tuple[list [array], float]: the new variable values :math:`x^{(t+1)}` and the objective
function output prior to the step.
If single arg is provided, list [array] is replaced by array.
Copy link
Member Author

Choose a reason for hiding this comment

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

Continuation lines in Returns: should never be indented; if they are, sphinx will treat them as a glossary entry and bold the first line.

@@ -106,8 +106,8 @@ def step(self, objective_fn, *args, grad_fn=None, **kwargs):

@staticmethod
def compute_grad(objective_fn, args, kwargs, grad_fn=None):
r"""Compute gradient of the objective_fn at the point x and return it along with the
Copy link
Member Author

Choose a reason for hiding this comment

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

The docstring conventions discourage the use of variable names in the first line/summary line of the docstring. Hence replacing objective_fn with 'objective function'.

@codecov
Copy link

codecov bot commented Jan 30, 2021

Codecov Report

Merging #1053 (ef482b1) into master (7229005) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1053   +/-   ##
=======================================
  Coverage   97.75%   97.75%           
=======================================
  Files         153      153           
  Lines       11514    11514           
=======================================
  Hits        11255    11255           
  Misses        259      259           
Impacted Files Coverage Δ
pennylane/optimize/gradient_descent.py 100.00% <ø> (ø)
pennylane/optimize/momentum.py 100.00% <ø> (ø)
pennylane/optimize/nesterov_momentum.py 100.00% <ø> (ø)
pennylane/optimize/qng.py 100.00% <ø> (ø)
pennylane/optimize/rotoselect.py 95.65% <ø> (ø)
pennylane/optimize/rotosolve.py 100.00% <ø> (ø)

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 7229005...be1627e. Read the comment docs.

@josh146 josh146 merged commit 4bd310f into master Jan 30, 2021
@josh146 josh146 deleted the fix-doc-bug branch January 30, 2021 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation 📘 Documentation changes and updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants