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

add how-to-guide on classical feedforward and control flow #198

Merged
merged 9 commits into from
Oct 26, 2023
Merged

add how-to-guide on classical feedforward and control flow #198

merged 9 commits into from
Oct 26, 2023

Conversation

kevinsung
Copy link
Collaborator

Fixes #33.

That issue mentioned that we would add a warning box on limitations and link to a page on limitations in the run section, which will be created as part of #148. In the end I decided to keep this guide self-contained and omit the warning. If we still want to add the warning box, we can do it as part of the latter issue.

@kevinsung
Copy link
Collaborator Author

The CI complains

      Invalid Jupyter notebook metadata. Every .ipynb file needs to 
      set 'title' and 'description' in the file metadata.

Can we make this more lenient? I created the notebook with VS Code and it opens in Jupyter Lab. Otherwise, what's an easy way to ensure notebooks pass the linter?

@Eric-Arellano
Copy link
Collaborator

Can we make this more lenient?

No, we must have the metadata set for the docs infra to work. I need to improve the instructions though, #200.

To fix, open the notebook in text mode in VSCode and scroll to the very bottom of the file to its metadata section - make sure it's metadata for the notebook and not an individual cell. Then add title and description. The title is the HTML page title, and description is a ~1 sentence summary.

Copy link
Collaborator

@javabster javabster left a comment

Choose a reason for hiding this comment

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

Overall looking great thanks @kevinsung!

I left some comments below, also could you please update the toc so this file will appear in the sidebar, @abbycross or @beckykd or @Eric-Arellano can help you if you don't know how to do this

docs/build/dynamic-circuits.ipynb Outdated Show resolved Hide resolved
@abbycross
Copy link
Collaborator

Overall looking great thanks @kevinsung!

I left some comments below, also could you please update the toc so this file will appear in the sidebar, @abbycross or @beckykd or @Eric-Arellano can help you if you don't know how to do this

I will add this to the toc according to our Build nav bar plan, and just FYI it is going to be first in the Build section since the pages above it haven't been published yet.

@kevinsung
Copy link
Collaborator Author

@javabster I've addressed your comments and updated the toc.

docs/build/_toc.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@javabster javabster left a comment

Choose a reason for hiding this comment

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

The return value of the context manager can be stored and used subsequently as a context manager to create an else block, which is executed whenever the contents of the if block are not executed.

Is there a way to rewrite this line without saying context manager twice? maybe something like "the measurement outcome of the if statement can be stored and used subsequently as a context manager to create an else block..."

Copy link
Collaborator

@javabster javabster left a comment

Choose a reason for hiding this comment

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

Note that for clarify, we chose

do you mean "for clarity"?

@javabster
Copy link
Collaborator

other than the small language things above this LGTM 🚀

@kevinsung
Copy link
Collaborator Author

I've updated the wording to

The with statement can be given an assignment target which is itself a context manager that can be stored and subsequently used to create an else block, which is executed whenever the contents of the if block are not executed.

and fixed the "clarity" typo.

@kevinsung kevinsung removed the request for review from jakelishman October 26, 2023 12:53
@kevinsung kevinsung merged commit 61636c1 into Qiskit:main Oct 26, 2023
4 checks passed
@kevinsung kevinsung deleted the dynamic-circuits branch October 26, 2023 12:59
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Fixes Qiskit#33.

That issue mentioned that we would add a warning box on limitations and
link to a page on limitations in the run section, which will be created
as part of Qiskit#148. In the
end I decided to keep this guide self-contained and omit the warning. If
we still want to add the warning box, we can do it as part of the latter
issue.

---------

Co-authored-by: abbycross <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[ NEW PAGE ] Build/ Control flow with Qiskit
4 participants