Skip to content
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

Merged
merged 12 commits into from
Jun 29, 2024
Merged

swap matrices, topocharge #61

merged 12 commits into from
Jun 29, 2024

Conversation

juli6nne
Copy link
Contributor

No description provided.

@Haadi-Khan Haadi-Khan requested a review from rbyrne299 June 27, 2024 22:16
Copy link
Contributor

@rbyrne299 rbyrne299 left a 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:
Copy link
Contributor

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
Copy link
Contributor

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

@rbyrne299 rbyrne299 merged commit e691a35 into main Jun 29, 2024
1 check passed
@rbyrne299 rbyrne299 deleted the braiding branch June 29, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants