-
Notifications
You must be signed in to change notification settings - Fork 56
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
WIP: Feature propagatable error gens #435
base: develop
Are you sure you want to change the base?
Conversation
Moved some files around, integrated stim translations into pyGSTi, Added a tutorial notebook to examples
def com(p1,p2): | ||
P1 = pyGSTiPauli_2_stimPauli(p1) | ||
P2=pyGSTiPauli_2_stimPauli(p2) | ||
P3=P1*P2-P2*P1 |
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.
@ashenmill Is there a specific version of STIM that is required for this? I tried subtracting PauliString objects like this on stim 1.13.0 and got unsupported operand type. Negation and addition do exist, but addition seems to just be concatenation. Am I missing something?
wT=ErG1.getWeight()*ErG2.getWeight()*weightFlip*BCHweight | ||
|
||
if ErG1.getType()=='H' and ErG2.getType()=='H': | ||
pVec=com(ErG1.getP1() , ErG2.getP2()) |
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.
Should this not be ErG2.getP1()
? Hamiltonian error generators should only have one basis element, right?
in this function, if the weight is needed please store paulistring::weight prior to applying this function | ||
''' | ||
def stimPauli_2_pyGSTiPauli(pauliString): | ||
return str(pauliString)[1:].replace('_',"I") |
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 line may fail if there is a complex phase because you would need to remove 2 characters.
MultiGate: lets the code know | ||
returns: list of propagatableerrorgens | ||
''' | ||
def ErrorPropagator(circ,errorModel,MultiGateDict={},BCHOrder=1,BCHLayerwise=False,NonMarkovian=False,MultiGate=False,ErrorLayerDef=False): |
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.
Quick note to future us to remove this mutable default arg for MultiGateDict.
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.
Just noting some discrepancies between the notes and code. It doesn't really matter if we know the code is correct, but we will eventually want some documentation for this so ensuring the notes match is probably a good step.
pVec=com(ErG2.getP1() , ErG1.getP1()) | ||
errorGens.append( propagatableerrorgen( 'C' , [ErG2.getP1() , pVec[1]] , -1j*wT *pVec[0] ) ) | ||
|
||
elif ErG1.getType()=='H' and ErG2.getType()=='C': |
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.
Notes have this as
but what is coded is
I think the swapped order of the commutator introduces a sign difference, right? Which is correct?
elif ErG1.getType()=='C' and ErG2.getType()=='H': | ||
errorGens = commute_errors(ErG2,ErG1,weightFlip=-1.0,BCHweight=BCHweight) | ||
|
||
elif ErG1.getType()=='H' and ErG2.getType()=='A': |
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.
Similar as [H,C]. The notes have
but coded is
Again, we have comm order swap in the first term with a potential sign difference. Which is correct?
errorGens = commute_errors(ErG2,ErG1,weightFlip=-1.0,BCHweight=BCHweight) | ||
|
||
elif ErG1.getType()=='S' and ErG2.getType()=='S': | ||
errorGens.append( propagatableerrorgen('H', ErG1.getP1(),0 )) |
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 do we add what looks like a dummy H label?
elif ErG1.getType()=='S' and ErG2.getType()=='S': | ||
errorGens.append( propagatableerrorgen('H', ErG1.getP1(),0 )) | ||
|
||
elif ErG1.getType()=='S' and ErG2.getType()=='C': |
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.
Notes have
but code is
Similar to the commutators, this change in multiplication could come with a sign change (depending on whether
(Also we are computing
errorGens.append( propagatableerrorgen( 'C', [pVec1[1], pVec2[1]] , -1j*wT*pVec1[0]*pVec2[0])) | ||
pVec1 = com(ErG2.getP1() , ErG2.getP2()) | ||
pVec2 = com(ErG1.getP1(),pVec1[1]) | ||
errorGens.append( propagatableerrorgen( 'A', [ErG1.getP1(), pVec2[1]] ,-.5*wT*pVec1[0]*pVec2[0])) |
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.
Note has
elif ErG1.getType() == 'A' and ErG1.getType() == 'S': | ||
errorGens = commute_errors(ErG2,ErG1,weightFlip=-1.0,BCHweight=BCHweight) | ||
|
||
elif ErG1.getType() == 'C' and ErG2.getType() == 'C': |
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.
First, I'll just say that this is way more readable with the intermediate variables. It would be even more readable if pVec*
were named things like AP
or acomAB
or things like that.
Note has
but coded up is
Which is correct? (Also we are recomputing anticommutators)
pVec1=com(P,Q) | ||
pVec2=com(A,B) | ||
pVec3=com(pVec1[1],pVec2[1]) | ||
errorGens.append( propagatableerrorgen('H',[pVec3[1]] ,.25*wT*pVec1[0]*pVec2[0]*pVec3[0])) |
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.
Notes have a factor of
errorGens.append( propagatableerrorgen('C', [pVec2[1] , Q ], .5*1j*wT*pVec1[0]*pVec2[0] )) | ||
pVec1 = acom(A,B) | ||
pVec2 =com(Q,pVec1[1]) | ||
errorGens.append( propagatableerrorgen('C',[pVec2[1],P ],-.5*1j*wT*pVec1[0]*pVec2[0] )) |
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.
Notes have
FYI, I have split off another branch from this called I'm not planning to make tools that cover all of the functionality Ashe covered, so likely some merge will be required, but I think my branch will have a pretty good idea of what these tools might look like when they are finally merged. I hope this will be at least a good discussion point for us in a week. :) |
…yGSTio/pyGSTi into feature-Propagatable-ErrorGens
Added Analytic Propagation
Created an initial implementation of the error_gen Object initial implementation. Also merged a few functions from the ml branch for compressed sensing purposes.
This is a draft PR for integrating @ashenmill's work on end-of-circuit error generators, which will probably be paired with some sort of STIM forward simulator support.
I'm opening this PR early so that I can comment on specific lines of code.