-
Notifications
You must be signed in to change notification settings - Fork 15
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
Entanglement simulation on a grid with custom predicates #90
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #90 +/- ##
==========================================
- Coverage 74.98% 71.11% -3.87%
==========================================
Files 37 37
Lines 1399 1475 +76
==========================================
Hits 1049 1049
- Misses 350 426 +76 ☔ View full report in Codecov by Sentry. |
It seems this is completely independent from #86 , right? I should be able to merge it shortly |
This looks pretty good. I left some minor comments. An example based on this should be added to the examples folder as well. |
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
Architecture: x86_64
Benchmark ResultJudge resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/QuantumSavory.jl/QuantumSavory.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed...[Comment body truncated] |
… predicate-true results
The findmin and findmax that were used in findswappablequbits do not make sense if we are not in a 1D chain, so they should also be parameterized. I think I did that correctly. Please review my changes, fix any issues that I have introduced to make the tests pass again and merge without waiting for me. The "Downgrade" is failing due to unrelated issues. |
The |
It returns an index, not the argument itself. Am I getting something wrong? |
src/ProtocolZoo/ProtocolZoo.jl
Outdated
_, il = findmin(n->n.tag[2], low_nodes) # TODO make [2] into a nice named property | ||
_, ih = findmax(n->n.tag[2], high_nodes) | ||
_, il = choose_low(n->n.tag[2], low_nodes) # TODO make [2] into a nice named property | ||
_, ih = choose_high(n->n.tag[2], high_nodes) |
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.
oh, you are saying I should have removed the underscore. Indeed, the underscore should not be there.
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.
No, in case of a vector of NamedTuple
the output is the NamedTuple
, the above snippet takes in a simple vector.
julia> using QuantumSavory; using QuantumSavory.ProtocolZoo: EntanglementCounterpart;
julia> reg = Register(5);
julia> a = [(slot=reg[9], tag=Tag(EntanglementCounterpart, 4, 1)), (slot=reg[8], tag=Tag(EntanglementCounterpart, 2, 5)), (slot=reg[4], tag=Tag(EntanglementCounterpart, 3, 1)), (slot=reg[1], tag=Tag(EntanglementCounterpart, 3, 1))]
4-element Vector{@NamedTuple{slot::RegRef, tag::Tag}}:
(slot = Slot 9, tag = Entangled to 4.1)
(slot = Slot 8, tag = Entangled to 2.5)
(slot = Slot 4, tag = Entangled to 3.1)
(slot = Slot 1, tag = Entangled to 3.1)
julia> argmax(n->n.tag[2], a)
(slot = Slot 9, tag = Entangled to 4.1)
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.
Ah, yes, it should be just il = choose_low(low_nodes)
. Then it all works, right? argmax
has way more signatures than I expected. With that change the random_index function should also work. Apologies for the only partial update.
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.
argmax
(argmin) returns the tuple: (slot = Slot 9, tag = Entangled to 4.1)
we can't use it to index the array of NamedTuple, so it doesn't work like that, if we want to do argmax, we would have to return il
and ih
directly
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.
argmax((n->n.tag[2] for n in low_nodes))
Start from scratch here: what is the point of this change -- it is to make it easy for a user to provide a chooser function that gives you the preferred node to select. You can not have such a function depend on the internals of your code, you can not expect the user to know of it. Apologies for the repeated typos though, those are on me.
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.
The change totally makes sense. The problem is just that it the way it was implemented before(using argmax
or argmin
), it didn't work with indexing.
The third comment suggests doing il = choose_low(low_nodes)
, that throws an error:
using QuantumSavory; using QuantumSavory.ProtocolZoo: EntanglementCounterpart;
reg = Register(5);
low_nodes = [(slot=reg[9], tag=Tag(EntanglementCounterpart, 4, 1)), (slot=reg[8], tag=Tag(EntanglementCounterpart, 2, 5)), (slot=reg[4], tag=Tag(EntanglementCounterpart, 3, 1)), (slot=reg[1], tag=Tag(EntanglementCounterpart, 3, 1))]
choose_low = argmin
choose_low(low_nodes)
ERROR: MethodError: no method matching isless(::RegRef, ::RegRef)
Closest candidates are:
isless(!Matched::Missing, ::Any)
@ Base missing.jl:87
isless(::Any, !Matched::Missing)
@ Base missing.jl:88
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.
Check my last comment. Does this work: argmax((n->n.tag[2] for n in low_nodes))
or something to that effect.
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.
The documentation comment also helps with this:
the `nodeL` predicate can return many positive candidates; `chooseL` picks one of them (by index into the array of filtered `nodeL` results), defaults to a random pick `arr->rand(keys(arr))`; if you are working on a repeater chain a good choice is `argmin`, i.e. the node furthest to the "left"
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.
I see, thanks, I got really confused in the beginning, sorry about that
|
||
## Custom Predicates | ||
|
||
function top_left(net, node, x) |
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.
I am waiting on CI and will then merge. However, this is an O(n) check for something that should be O(1).
From a node index get its x y coordinates (currently you get only one of them).
Then do the same for the other node.
Then just compare them to verify that you are to the left (one comparison) and to the top (second comparison).
No need for a for loop.
Submit a PR with the fix and feel free to merge it. It should also make this function much simpler.
check the commit I added for some small cleanup changes I made |
Following #85, here, an entanglement simulation is implemented on a grid that runs
EntanglerProt
andSwapperProt
only on the diagonal