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

Update getting started notebook #5046

Merged
merged 3 commits into from
Mar 10, 2022
Merged

Update getting started notebook #5046

merged 3 commits into from
Mar 10, 2022

Conversation

maffoo
Copy link
Contributor

@maffoo maffoo commented Mar 1, 2022

  • Use Circuit.transform_qubits instead of cg.optimize_for_sycamore, since all we need to do is change qubits.
  • Don't specify a gate_set, which will use CircuitSerializer by default.
  • Use standard string representation of cirq.Result.
  • Remove unused imports.

- Use Circuit.transform_qubits instead of cg.optimize_for_sycamore.
- Don't specify a gate_set, which will use CircuitSerializer by default.
- Use standard string representation of cirq.Result.
- Remove unused imports.
@maffoo maffoo requested review from wcourtney, a team, vtomole, cduck and verult as code owners March 1, 2022 21:50
@maffoo maffoo requested a review from viathor March 1, 2022 21:50
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@CirqBot CirqBot added the size: S 10< lines changed <50 label Mar 1, 2022
@verult
Copy link
Collaborator

verult commented Mar 2, 2022

lgtm. transform_qubits could eventually be replaced with a decompose using a CompilationTargetGateset.

Will leave approval to someone more familiar with transformers

Comment on lines +395 to +396
"# Transform circuit to use an available hardware qubit.\n",
"hw_circuit = circuit.transform_qubits(lambda q: valid_qubit)\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

While I agree with not using optimize_for_gateset since it breaks WYSIWYG, I worry that removing gateset considerations entirely from this tutorial will leave readers stranded as soon as they attempt something outside the HW gateset.

CC @tanujkhattar to recommend a WYSIWYG-friendly transformer for this, if one exists.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of using a transformer, we could point them to further documentation for non-native gates. Either here or in the "Next steps" section at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We currently have a next step "Explore best practices for getting circuits to run on hardware." That seems like it would be a natural places to talk about transformers that can compiler to native gates. It currently talks about validation and optimized_for_sycamore, so this would need to be updated to reflect recent development.

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 to Matthew's recommendation - if we update that doc to cover this detail (here or in a follow-up), I'd be happy to sign off on the changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the test to talk about transform_qubits instead of optimized_for_sycamore and added a note about transforming to available gates with a link to the best practices guide. I couldn't see any guides about transformers or compilation target gatesets, but once we have something we can link to that instead.

@95-martin-orion 95-martin-orion added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Mar 10, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Mar 10, 2022
@CirqBot CirqBot merged commit d967317 into master Mar 10, 2022
@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Mar 10, 2022
@CirqBot CirqBot deleted the u/maffoo/start-optimize branch March 10, 2022 18:01
@CirqBot CirqBot removed the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Mar 10, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
- Use Circuit.transform_qubits instead of cg.optimize_for_sycamore, since all we need to do is change qubits.
- Don't specify a gate_set, which will use CircuitSerializer by default.
- Use standard string representation of cirq.Result.
- Remove unused imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: S 10< lines changed <50
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants