-
Notifications
You must be signed in to change notification settings - Fork 0
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
swap matrices, topocharge #61
Conversation
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.
Looks great, thanks
Just need to fix pairs_to_indices in Simulator & ideally write test for it, then should be good to go
""" | ||
return self._anyons | ||
|
||
def get_state(self) -> State: |
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 know I said I was gonna add state to simulator so I'll edit anyways, but just FYI this get_state method makes it sound like there's already a state field that you're "getting". Classes often have "getter & setter" methods for this purpose, but that doesn't apply here, so a clearer name would've been smth like init_state. Not big deal though
index_1 = self.get_anyon_index(anyon_A) | ||
index_2 = self.get_anyon_index(anyon_B) | ||
anyon_indices.append((index_1, index_2)) | ||
return anyon_indices |
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 return statement should be outside the for loop, otherwise the loop will never go beyond the first iteration. the fact that your PR passed check/tests means that this function wasn't covered anywhere in the existing tests, so might be good to test this somewhere
No description provided.