refactor: update T factory example #817
Conversation
|
Instead of deleting, could you move the notebook to the tests module? We still should test that the notebook works in CI |
CalMacCQ
changed the title
Would you prefer I left it as a notebook file in the |
|
Sounds good 👍 You will also need to update the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #817 +/- ##
==========================================
+ Coverage 93.07% 93.16% +0.08%
==========================================
Files 72 72
Lines 8494 8528 +34
==========================================
+ Hits 7906 7945 +39
+ Misses 588 583 -5 ☔ View full report in Codecov by Sentry. |
Done, thanks :) |
examples/t_factory.py
Outdated
| target: qubit @owned, q0: qubit @owned, q1: qubit @owned, q2: qubit @owned, q3: qubit @owned | ||
| target: qubit @ owned, | ||
| q0: qubit @ owned, | ||
| q1: qubit @ owned, |
There was a problem hiding this comment.
An alertnative would be to take an array of qubits as input here and index into it for the cz gates. Do you think this is better? Tried it locally and it works.
There was a problem hiding this comment.
added in c6e96b5. I can revert if the old way is preferred.
|
Should there be automated testing to catch API breaks in these example scripts? |
|
They are already being tested in |
|
But we should probably exclude them from coverage since the Guppy code is not actually being executed |
examples/t_factory.py
Outdated
| def prepare_approx(q: qubit @owned) -> qubit: | ||
| q = ry(q, angle(py(phi))) | ||
| return rz(q, pi / 4) | ||
| def prepare_approx(q: qubit @ owned) -> qubit: |
There was a problem hiding this comment.
Imo it would be nicer to allocate the qubit in this function instead of passing it?
There was a problem hiding this comment.
Good idea, done
See 874c4e9
tests/demo.ipynb
Outdated
There was a problem hiding this comment.
Could you move the file to tests/integration and maybe into its own directory notebooks?
There was a problem hiding this comment.
Makes sense. see e92aab9
examples/t_factory.py
Outdated
| target: qubit @owned, q0: qubit @owned, q1: qubit @owned, q2: qubit @owned, q3: qubit @owned | ||
| target: qubit @ owned, | ||
| q0: qubit @ owned, | ||
| q1: qubit @ owned, |
closes #813
Replacing all
listusage in thet_factoryexample.I've used arrays in the
distillfunction and anOption[qubit]return int_statewhich I think is much nicer.