-
Notifications
You must be signed in to change notification settings - Fork 49
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
implement sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT*tId1
, P"-Y"*sm
and copy(sm)
from #260
#316
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #316 +/- ##
==========================================
- Coverage 82.97% 82.89% -0.08%
==========================================
Files 61 61
Lines 4023 4069 +46
==========================================
+ Hits 3338 3373 +35
- Misses 685 696 +11 ☔ View full report in Codecov by Sentry. |
I hope the TODOs are satisfactory. After the TODOs, I will again mark the PR as draft. Attaching the benchmark results for TODO1
|
The failure of the downgrade and build test is due to stuff not related to this PR, It has been pointed out in the issue 299 and 245. As you said back then (i245) that it's a statistical test which fails sometime in a thousand runs but recently, it's been failing quite often.
|
What's changed in I sought some advice on your question/concern: "Is this actually necessary? Is it actually dangerous to modify the dictionary keys while iterating over it?" After discussions, it turned out that this is dangerous and Valentin provided good reasons (forwarded them) of why it's dangerous and suggested that
I have made refined further by using in-built Julia documentation for Using
I hope this is satisfactory. |
In order to become more familiar with non-clifford requirements, I went through some TODOs to understand your cool work so that I can ask better questions. The first goal was first to wrap my head around your implementation by doing TODOs. I hope that the latest TODOs improvements are satisfactory. Besides, I have also read Ted's paper as well (till the 3 algorithms section) so that I can follow the concepts used here. Before answering questions, it is necessary for me to describe all the context to ask reasonable, efficient questions. Please skip to questions directly if you like. Please take your time answering them at your convenience. Given that:
Algorithms:
Going on a tangent:
I see the rowdecompose utility I am also going through project_reset_trace.jl to understand the Questions: In my rough estimate, the
I hope that I raised reasonable questions that you would be expecting from someone who is working on non-clifford. Maybe some of these questions are bad. I think that maybe addressing all the TODOs would be helpful to detect why is Kindly please take your time answering them. In the meantime, I will read the Ted's paper again, and will correspond reading with the implementation. Thank you! |
Is this for review or still WIP? |
Technically, it's WIP. I left it as open because I am looking for your comments on the TODOs. Thanks for your help. In the meantime, I will turn it into draft. |
Sorry, I will not be able to provide particularly detailed answers to this set of questions -- to some extent the point of the bounty is for the contributor to work through the paper and code on their own. Happy to attempt to answer the occasional brief question though. |
No problem. I totally understand. The set of questions helped me keep track of points, and eventually reading the paper frequently helped me refer to them :) It's like keeping a scratch space, although I should note them down locally on text document. I did realize that I needed to answer brief questions specific to one particular thing.... not essay questions! |
indeed, the occasional brief question is much more probable to get a quick answer (and to not get lost) |
Kindly please have a look at implementation of I'm unsure if this is feasible, so I'd value your input before committing. Algorithm 2 (page 8 of Ted's paper) It covers both two cases for I understand that atm, it's only updating I am unsure whether algorithm Applied the same test of
Some runs:
Conjugate Destabs (
Some Runs
|
sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT⊗tId1
and copy(sm)
from #260
sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT⊗tId1
and copy(sm)
from #260sm⊗sm
, tHadamard*sm
, pcT*P"X"
, pcT⊗tId1
and copy(sm)
from #260
sm⊗sm
, tHadamard*sm
, pcT*P"X"
, pcT⊗tId1
and copy(sm)
from #260sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT*tId1
and copy(sm)
from #260
Apologies for ALL the inconvenience. This PR is rewired to complete the following tasks from #260 , continuing the work from #259. These tasks are as follows:
Hence, dividing the nonclifford functionality into smaller PRs. This helps me not to get lost by taking definite smaller steps. Hopefully, this approach is satisfactory. I will focus on the doctests, and tests for these, and will request review in a day or two. Given that:
Outputs are attached as follows: sm⊗sm
tHadamard*sm
pcT⊗P"X"
copy(sm)
tId1*pcT
|
sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT*tId1
and copy(sm)
from #260sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT*tId1
, pcT *sm
, P"-Y"*sm
and copy(sm)
from #260
sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT*tId1
, pcT *sm
, P"-Y"*sm
and copy(sm)
from #260sm⊗sm
, tHadamard*sm
, pcT⊗P"X"
, pcT*tId1
, P"-Y"*sm
and copy(sm)
from #260
…by completing some interfaces
The non-cliff interfaces mentioned in this comment: #316 (comment) are doctested along basic tests. I have squashed all the commets related to these functionalities in this one commit for review: a6c7b14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Fe-r-oz , you mentioned that the commits are squashed to make it more straightforward to review, but that does not seem to be the case. If you look at the list of commits, there is a lot of them, not organized in any particular way: https://github.com/QuantumSavory/QuantumClifford.jl/pull/316/commits
Could you make a new pull request from this branch towards the nonclif branch (i.e. towards this PR #259) so that it is easy to look only at the changes you have made (and please make sure the commits are organized).
Apologies for this frustrating experience. I only squashed the last 5 commits not all. I was unable to squash all of them because of some commits |
Closing this PR of it's making the review quite unfeasible. |
This PR aims to address some TODOs to become familiar with @Krastanov's very cool work on non-clifford functionality with the goal to resolve #309 by completing the non-clifford functionalities.