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

Improve Quickstart: move optimizer and config sections #2608

Merged
merged 13 commits into from
Jun 9, 2022

Conversation

mariaschuld
Copy link
Contributor

The Optimizer and Configuration sections were very sparsely populated, and I integrated them into others.

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.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.

@mariaschuld mariaschuld requested a review from josh146 May 24, 2022 13:09
@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #2608 (85509d3) into master (6d63dfd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2608   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files         249      249           
  Lines       20256    20256           
=======================================
  Hits        20173    20173           
  Misses         83       83           
Impacted Files Coverage Δ
pennylane/qnn/cost.py 100.00% <ø> (ø)
pennylane/vqe/vqe.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 6d63dfd...85509d3. Read the comment docs.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks @mariaschuld!

I have to admit I find this one a bit hard to review, since it is mostly on orgnanization, so I'll just leave a top-level comment.

  • Configuration: I agree this should be removed from the sidebar 👍 It was very niche, and distracted from the other entries.

    However, I would also say it doesn't fit in the Circuits page?

    • It makes the circuit page very long, which negates one of the advantages of the quickstarts (they are concise reference pages)

    • I'm also not sure how important it is, and whether anyone clicking on Circuits would be interested in configuration?

    Perhaps another solution would be to keep Configuration as a separate ReST page, but still remove it from the ToC. Instead, we could have a sentence in the Circuits/Defining a Device section that says 'For details on saving device configurations, please see Configurations.'

  • Optimizers: With the optimizer page, I am a bit more conflicted. I agree that it belongs to some extent within the NumPy page, since they are only applicable if you are using Autograd. On the other hand, the quantum-aware optimizers (QNG, Rotosolve, ShotAdaptive, LieAlgebra). My condensed thoughts:

    • I think are very important features, and shouldn't be so buried (at the moment, it takes 3 clicks, scrolling, and clicking on a picture of the NumPy logo to find them!).

    • I imagine that these are important enough features that users may be likely to switch from TF/Torch to Autograd to use them if they need them.

    • They are actually possible to use with JAX (but it is less trivial, you need to pass the grad_fn manually).

    I'm not sure what the best solution is here 🤔 I think they would be good to keep them more discoverable, while also continuing to list them in the NumPy page?

@mariaschuld
Copy link
Contributor Author

Thanks for the comments. Making the config a linked but not listed stub is a great idea.

About the quantum-aware optimisers, they are referenced quite prominently in the note of the Gradients and Training page, no? That would be only one click?

Before, we were literally doubling up on this information: If I want to learn about how to optimise a circuit, do I go to that section or to optimizers? And then if I go to optimisers, I have to understand about interfaces to parse the list there. So I would argue they belong 100% on the Gradients and Training page?

Is there any way we can make them even more prominent, but move them to the logical section?

I'm sure @christina also has a good idea!

@mariaschuld mariaschuld requested a review from josh146 May 26, 2022 12:34
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks @mariaschuld! Looks like there are still a few links to the deleted optimizers page that Sphinx is complaining about:

image

Regarding doubling up on the optimizers: you are right. I just can't stop thinking that they are harder to find now than before, if you don't know where to look 🤔

Another option I can think of (that is still not perfect) is to have an 'Optimizers' subsection on the 'Gradients and Training' page, rather than a notebox, that lists the quantum aware optimizers using autosummary:

image

doc/introduction/interfaces.rst Outdated Show resolved Hide resolved
doc/introduction/interfaces/numpy.rst Show resolved Hide resolved
@mariaschuld mariaschuld requested a review from josh146 June 6, 2022 14:09
@mariaschuld mariaschuld merged commit 1e4dce8 into master Jun 9, 2022
@mariaschuld mariaschuld deleted the improve-quickstart7 branch June 9, 2022 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants