-
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
Added QuantumChannel #53
Conversation
Codecov Report
@@ Coverage Diff @@
## master #53 +/- ##
==========================================
+ Coverage 63.29% 65.90% +2.60%
==========================================
Files 27 28 +1
Lines 1079 1100 +21
==========================================
+ Hits 683 725 +42
+ Misses 396 375 -21
... and 6 files with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
Looking at the examples in ConcurrentSim, I do not think that |
Thank you for all of this! I left a bunch of comments in. Some of them are fairly nit-picky -- it is so that we can establish some coding style conventions that might slowly become actual APIs. My nit-pickiness will diminish in future pull requests. |
Please feel free to be as detailed with the reviews as you'd like, it can only help with improving the code quality of the repo, besides helping me code better ; ) |
As you make changes, could you click "resolve" on the comments that are already dealt with. It makes it a bit easier to read through. |
src/quantumchannel.jl
Outdated
end | ||
|
||
channel_reg = Register(qc) | ||
swap!(rref, channel_reg[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.
does channel_reg
has its time
value correctly set after the swap!
?
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.
Do you mean the accesstimes
attribute? No, it only swaps the staterefs
and stateindices
of the RegRef
s, should we edit the current swap! to do so?
src/quantumchannel.jl
Outdated
swap!(rref, channel_reg[1]) | ||
|
||
if !isnothing(qc.background) | ||
uptotime!(channel_reg[1], qc.queue.delay) |
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.
If swap!
properly updates the time
value, then this should be qc.queue.delay+now(qc.queue.store.env)
src/quantumchannel.jl
Outdated
if isassigned(rref) | ||
error("A take! operation is being performed on a QuantumChannel in order to swap the state into a Register, but the target register slot is not empty (it is already initialized).") | ||
end | ||
swap!(channel_reg[1], rref) |
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.
same question about swap!
properly updating time here?
I was unhappy with how cumbersome it was to use I edited the docstring a bit: (1) to remove implementation details and focus on the "why" (2) I think one of the doctests would have failed because it contained an arbitrary memory address. But (2) also showed me that some tests are not running, so I double checked a few things there... We have not been checking doctests for a while. Lets make buildkite a priority. |
Added QuantumChannel using
DelayQueue
fromConcurrentSim.jl
.