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

inv implementation for SingleQubitOperator #314

Merged
merged 15 commits into from
Jul 17, 2024

Conversation

Fe-r-oz
Copy link
Contributor

@Fe-r-oz Fe-r-oz commented Jul 12, 2024

Functionality that we want

julia> inv_op = inv(SingleQubitOperator(sHadamard(1)))
sHadamard on qubit 1
X₁ ⟼ + Z
Z₁ ⟼ + X

julia> expected_inv_op = sHadamard(1)
sHadamard on qubit 1
X₁ ⟼ + Z
Z₁ ⟼ + X

julia> inv(sHadamard(1))
sHadamard on qubit 1
X₁ ⟼ + Z
Z₁ ⟼ + X

I will add the twoqubit invs today as well. Hope you find this plausible!

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jul 12, 2024

Locally the tests passes: I hope it will pass here as well.

julia> include("test_symcliff.jl")
Precompiling QuantumClifford
  1 dependency successfully precompiled in 27 seconds. 46 already precompiled.
Test Summary:            | Pass  Total  Time
Small symbolic operators |  270    270  2.7s
Test Summary:             | Pass  Total  Time
Convert between small ops |   59     59  2.4s
Test Summary:                   | Pass  Total  Time
SingleQubitOperator Inv methods |   14     14  0.1s
Test.DefaultTestSet("SingleQubitOperator Inv methods", Any[], 14, false, false, true, 1.720769482610657e9, 1.720769482702756e9, false, "/home/Desktop/New/inv/QuantumClifford.jl/test/test_symcliff.jl")

Copy link
Member

@Krastanov Krastanov left a comment

Choose a reason for hiding this comment

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

a couple of notes

To make it easier for reviewers, could you also either mark unfinished pull requests as "draft"? (you are mentioning that you will be adding more to this one).

It is also a good idea to generally make small pull requests that do only one thing, as I would be much quicker to review such pull requests. So you can for instance just deal with single qubit gates in this pull request and submit the multi-qubit gate later. That makes reviewing and merging drastically easier.

test/test_symcliff.jl Outdated Show resolved Hide resolved
src/symbolic_cliffords.jl Outdated Show resolved Hide resolved
@Krastanov
Copy link
Member

Locally the tests passes: I hope it will pass here as well.

Thanks! Generally, do not worry about reporting stuff like that. I will anyway wait for the CI to run, so no need to investigate things unless there is a problem.

@Fe-r-oz Fe-r-oz marked this pull request as draft July 12, 2024 12:36
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jul 12, 2024

a couple of notes

Thank you for these notes!

@Fe-r-oz Fe-r-oz changed the title inv implementation for all subtypes of AbstractCliffordOperator inv implementation for all subtypes of AbstractSingleOperator Jul 12, 2024
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.96%. Comparing base (59e399d) to head (5280f11).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #314      +/-   ##
==========================================
- Coverage   82.97%   82.96%   -0.01%     
==========================================
  Files          61       61              
  Lines        4023     4033      +10     
==========================================
+ Hits         3338     3346       +8     
- Misses        685      687       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Fe-r-oz Fe-r-oz changed the title inv implementation for all subtypes of AbstractSingleOperator inv implementation for SingleQubitOperator Jul 12, 2024
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jul 12, 2024

Thanks for your comments. Indeed, they helped to polish the PR. I hope this is alright now!

@Fe-r-oz Fe-r-oz marked this pull request as ready for review July 12, 2024 13:28
@Fe-r-oz Fe-r-oz requested a review from Krastanov July 12, 2024 13:29
src/symbolic_cliffords.jl Outdated Show resolved Hide resolved
src/symbolic_cliffords.jl Outdated Show resolved Hide resolved
test/test_symcliff.jl Outdated Show resolved Hide resolved
@Fe-r-oz Fe-r-oz marked this pull request as draft July 12, 2024 14:14
@Fe-r-oz Fe-r-oz marked this pull request as ready for review July 12, 2024 17:57
src/symbolic_cliffords.jl Outdated Show resolved Hide resolved
src/symbolic_cliffords.jl Outdated Show resolved Hide resolved
src/symbolic_cliffords.jl Outdated Show resolved Hide resolved
test/test_symcliff.jl Outdated Show resolved Hide resolved
test/test_symcliff.jl Outdated Show resolved Hide resolved
@Fe-r-oz Fe-r-oz requested a review from Krastanov July 13, 2024 08:20
@Krastanov Krastanov merged commit 10bb4b6 into QuantumSavory:master Jul 17, 2024
15 checks passed
@Krastanov
Copy link
Member

Thanks for the contribution!

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jul 18, 2024

Thanks for the feedback!

@Fe-r-oz Fe-r-oz deleted the methods branch July 18, 2024 00:21
Fe-r-oz added a commit to Fe-r-oz/QuantumClifford.jl that referenced this pull request Sep 9, 2024
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.

2 participants