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

Functional style plaquette #203

Merged
merged 6 commits into from May 3, 2024

Conversation

nelimee
Copy link
Contributor

@nelimee nelimee commented Apr 10, 2024

Goal of the PR

Per this comment from @inmzhang and the discussion/poll that followed it seems like the active developers in this repository prefer a functional-style rather than an inheritance based style. This means that instead of

class Plaquette:
    pass

class EmptyPlaquette(Plaquette):
    pass

class ZInitialisationPlaquette(Plaquette):
    pass

# many more

with a Plaquette class whose sole purpose is to provide a common interface between plaquette, everyone voted for a functional-style:

class Plaquette:
    pass

def empty_plaquette(qubits: PlaquetteQubits) -> Plaquette:
    pass

def z_initialisation_plaquette(qubits: PlaquetteQubits) -> Plaquette:
    pass

# many more

with Plaquette being the only class used, and sub-classes are replaced by functions that initialise Plaquette instances.

How was it done?

This change is mostly (entirely?) cosmetic, as the sub-classes of Plaquette were only initialising correctly their Plaquette subclasses. This shows quite well in this PR: the changes are quite small and can be summarised in 2 points:

  • Change the subclass name by the function name. The new functions have exactly the same parameters as their corresponding Plaquette subclass, so this change can be done using a "dumb" tool (i.e., just search & replace, no need for code-aware tools).
  • Change the subclasses to functions:
    class [CLASSNAME](Plaquette):
        def __init__(self, [PARAMETERS]):
            [CODE]
            super().__init__([SUPER_PARAMETERS])
    becomes
    def [SNAKE_CASE(CLASSNAME)](PARAMETERS) -> Plaquette:
        CODE
        return Plaquette([SUPER_PARAMETERS])
    with a slight variation for classes that were 2 or more levels under Plaquette in the inheritance hierarchy (e.g. ZRoundedInitialisationPlaquette -> ZInitialisationPlaquette -> Plaquette) for which the super().__init__ call should be replaced with their superclass corresponding function.

@nelimee nelimee added enhancement New feature or request, may not be in the task flow backend Issue pertaining to the Python backend (tqec package) refactor Requires major updates to code base QOL Improves usability and functionality labels Apr 10, 2024
@nelimee nelimee self-assigned this Apr 10, 2024
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@nelimee nelimee linked an issue Apr 10, 2024 that may be closed by this pull request
@smburdick
Copy link
Contributor

Since this seems like a large change, please make the following updates:

  • Update the PR description to be more descriptive as to what is changing, and how.
  • Please update the documentation. This will involve writing a short wiki that describes how to use the plaquette APIs in the backend

@nelimee
Copy link
Contributor Author

nelimee commented Apr 16, 2024

Update the PR description to be more descriptive as to what is changing, and how.

Done

Please update the documentation. This will involve writing a short wiki that describes how to use the plaquette APIs in the backend

In my opinion, this would be better in the package documentation (see #109)

@nelimee nelimee mentioned this pull request Apr 26, 2024
4 tasks
@nelimee
Copy link
Contributor Author

nelimee commented May 2, 2024

Pinging for review, I would like this branch to be merged if you are still convinced that this is the right way to go. Feel free to comment on anything like @smburdick did, this is helpful to see if I missed anything.

@afowler
Copy link

afowler commented May 2, 2024 via email

Copy link
Contributor

@inmzhang inmzhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nelimee nelimee merged commit b990e8d into QCHackers:main May 3, 2024
5 checks passed
@nelimee nelimee deleted the functional_style_plaquette branch May 3, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Issue pertaining to the Python backend (tqec package) enhancement New feature or request, may not be in the task flow QOL Improves usability and functionality refactor Requires major updates to code base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change OOP-style Plaquette hierarchy to functional-style
4 participants