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

Fit new notebooks into existing content #1160

Merged
merged 6 commits into from
Apr 18, 2024
Merged

Conversation

frankharkins
Copy link
Member

PRs #977 and #1099 added some new pages in anticipation of a restructure which has since been postponed. This PR fits those pages into our current content structure.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@qiskit-bot
Copy link
Contributor

One or more of the the following people are requested to review this:

Copy link
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Nice! The notebooks look good and are appropriate in the new sections.

docs/run/_toc.json Outdated Show resolved Hide resolved
scripts/nb-tester/test-notebook.py Show resolved Hide resolved
Copy link
Collaborator

@kaelynj kaelynj left a comment

Choose a reason for hiding this comment

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

This all looks good to me! The only thing I think might be missing is an explanation of the PubResult data types that are return from the sampler and estimator somewhere on the Run page. Though perhaps that would fit into a separate issue.

@frankharkins
Copy link
Member Author

Good point, I've made #1194 to track that.

Copy link
Collaborator

@kaelynj kaelynj left a comment

Choose a reason for hiding this comment

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

Good point, I've made #1194 to track that.

Great! Looks good to ship 💯

@frankharkins frankharkins added this pull request to the merge queue Apr 18, 2024
}
]
},
{
"title": "Quantum Serverless workloads",
"url": "/run/quantum-serverless"
},
{
"title": "Visualize results",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should go above quantum serverless in the side bar

@frankharkins frankharkins removed this pull request from the merge queue due to a manual request Apr 18, 2024
@frankharkins frankharkins enabled auto-merge April 18, 2024 14:44
@frankharkins frankharkins added this pull request to the merge queue Apr 18, 2024
Merged via the queue into main with commit 7672590 Apr 18, 2024
5 checks passed
@frankharkins frankharkins deleted the FH/move-notebooks branch April 18, 2024 14:50
Copy link
Collaborator

Choose a reason for hiding this comment

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

How does output work for this file? It doesn't seem like it'd be deterministic when we rerun the notebook, or if a local user runs with a different account?

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 output will be deterministic when running using my account (which we also use in our actions). Users won't have access to this job but I think that's ok, plus there's no way to demonstrate retrieving a job without picking one from my account.

@@ -32,7 +32,7 @@
]
NOTEBOOKS_THAT_SUBMIT_JOBS = [
"docs/start/hello-world.ipynb",
"docs/analyze/saving-and-retrieving.ipynb",
"docs/run/save-and-retrieve.ipynb",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Outdated

github-merge-queue bot pushed a commit that referenced this pull request Apr 18, 2024
Fix oversight in #1160. This notebook no longer submits jobs so can be
removed from the list.
github-merge-queue bot pushed a commit that referenced this pull request Apr 19, 2024
This page originally submitted a simple job using the cloud simulator as
a demonstration. Since the simulators are being deprecated, #1160
removed the job-submitting step and hardcoded the `job_id` into the
notebook. This was a bad idea because it means the notebook breaks if
not run with my IBM Quantum account.

This PR reorganises the page to get `job_id` programatically first. This
lets us avoid hardcoding the `job_id` in the notebook, which means we
can run the notebook with any IBM account that has submitted at least
one job.

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
frankharkins added a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
PRs Qiskit#977 and Qiskit#1099 added some new pages in anticipation of a restructure
which has since been postponed. This PR fits those pages into our
current content structure.
frankharkins added a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Fix oversight in Qiskit#1160. This notebook no longer submits jobs so can be
removed from the list.
frankharkins added a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
This page originally submitted a simple job using the cloud simulator as
a demonstration. Since the simulators are being deprecated, Qiskit#1160
removed the job-submitting step and hardcoded the `job_id` into the
notebook. This was a bad idea because it means the notebook breaks if
not run with my IBM Quantum account.

This PR reorganises the page to get `job_id` programatically first. This
lets us avoid hardcoding the `job_id` in the notebook, which means we
can run the notebook with any IBM account that has submitted at least
one job.

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants