-
Notifications
You must be signed in to change notification settings - Fork 59
Add rotation cost model #461
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
Conversation
|
||
|
||
@frozen | ||
class LogarithmicModel(RotationCostModel): |
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.
This name makes sense here, where you can clearly see that it's an instance of RotationCostModel
but I suspect if it shows up outside of the class definition it will lose this critical context. Should probably have rotation in the name somewhere
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.
ptal
|
||
|
||
@frozen | ||
class ConstantCostWithPreparation(RotationCostModel): |
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.
again, in actual use the fact that this is for rotations is lost; but we don't want it to be too long.
ConstOverheadRotationCost
or something like that
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.
ptal
|
||
Attributes: | ||
bitsize: Number of digits of accuracy for approximating a rotation. | ||
slope: The coefficient of $log_2{budget}$. |
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.
why is there a slope? isn't this supposed to be constant?
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.
for computing the cost of the overhead rotation
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 changed the signature to make this clearer
) | ||
|
||
|
||
BeverlandEtAl = LogarithmicModel( |
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.
Outside of this definition, this variable will appear without the context that it's a rotation cost model
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.
updated
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.
👍
Can you speak more about this? |
@mpharrigan to use TComplexity I will have to convert the toffolis to Ts The rotations are implemented in Toffolis so I think the runtime estimate will defer between an architectures that synthesize magic T state vs magic CCZ states. |
oh for some reason I thought we had a toffoli/ccz field in the |
This abstraction covers both standarad models which have a cost of$A \log_2(1/\epsilon) + B$ Where $A$ $B$ depend on the gateset, approximation protocol and hardware parameters and are obtained by fitting regression models. It also covers models with constant per rotation of cost which require a circuit that gets prepared once which gives preparation overhead.
and
I'm reporting the cost as an
AlgorithmSummary
object rather than aTComplexity
object because it's more natural to report the cost of the second model in terms of Toffoli count rather than T count.Related to #370 and is part of #269