grid entanglement with multiple paths #106
Conversation
Abhishek-1Bhatt
added
the
Skip-Changelog
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #106 +/- ##
==========================================
+ Coverage 71.14% 71.42% +0.28%
==========================================
Files 37 37
Lines 1473 1491 +18
==========================================
+ Hits 1048 1065 +17
- Misses 425 426 +1 ☔ View full report in Codecov by Sentry. |
|
I should have more time to look into this tonight if you have a chance to address my comments. |
… ::Tag)` bug, move filo to keyword argument
| isnothing(i) ? nothing : (;slot=reg[i], tag=tag) | ||
| end | ||
| function query(reg::Register, tag::Tag, ::Val{allB}=Val{false}(); locked::Union{Nothing,Bool}=nothing, assigned::Union{Nothing,Bool}=nothing, filo::Bool=true) where {allB} | ||
| _query(reg, tag, Val{allB}(), Val{filo}(); locked=locked, assigned=assigned) |
There was a problem hiding this comment.
this indirection from a Bool keyword argument to a Val{Bool} argument, is necessary if we want the compiler to guarantee that separate machine code is compiled for each of the two values (true / false).
It is also a way to avoid code duplication (e.g. queryall simply calls query(...,Val(true)))
It made sense when the function was simpler and it makes sense for private things.
But for public things like the filo keyword, it is cumbersome to use; Moreover, it is not simple anymore, so we should probably just split these methods, instead of having all these compile-time if/else statements (in Julia an if-else that depends only on information in the type system, will be decided during compilation and there will be no if/else actually happening when we run the program)
| @test query(reg, EntanglementCounterpart, 2, 22) == (slot = reg[2], depth = 7, tag = Tag(EntanglementCounterpart,2,22)) | ||
| @test queryall(reg, EntanglementCounterpart, 2, 22) == [(slot = reg[2], depth = 7, tag = Tag(EntanglementCounterpart,2,22)), (slot = reg[2], depth = 2, tag = Tag(EntanglementCounterpart,2,22))] | ||
| @test queryall(reg, Tag(EntanglementCounterpart, 2, 22)) == [(slot = reg[2], depth = 7, tag = Tag(EntanglementCounterpart,2,22)), (slot = reg[2], depth = 2, tag = Tag(EntanglementCounterpart,2,22))] | ||
| @test queryall(reg, EntanglementCounterpart, 2, 22) == queryall(reg, EntanglementCounterpart, ==(2), ==(22)) == queryall(reg, Tag(EntanglementCounterpart, 2, 22)) | ||
| @test queryall(reg, Tag(EntanglementCounterpart, 2, 22); filo=false) == reverse([(slot = reg[2], depth = 7, tag = Tag(EntanglementCounterpart,2,22)), (slot = reg[2], depth = 2, tag = Tag(EntanglementCounterpart,2,22))]) | ||
| @test queryall(reg, EntanglementCounterpart, 2, 22; filo=false) == queryall(reg, EntanglementCounterpart, ==(2), ==(22); filo=false) == queryall(reg, Tag(EntanglementCounterpart, 2, 22); filo=false) |
There was a problem hiding this comment.
all of these were failing before I fixed the issue with queryall(::Register, ...)
|
@Abhishek-1Bhatt , thank you, this is merged now! Please check out the changes I made, in terms of consistency of interface or ease of reading of tests. Please learn more about how rebasing works in git and github. It is crucially important to get more comfortable with it, as it is the only way to scalably work on projects like this: a follow-up PR can not wait on this to be finished, otherwise work will never be done. Rebasing takes care tracking code changes. Testing should always be so thorough that it takes care of worries about how changes affect other code (even if we were not using much rebasing). |
|
Oh, I also marked a todo in one of the test files -- currently the protocols are tested only for a small finite number of rounds. That can hide many bugs -- it is very important to have tests where the number of rounds is not constrained. You are probably already investigating this in your other PRs, but it is worth mentioning it here. |
Builds on #90