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

Adds AbstractInitialMapper base class and IdentityInitialMapper #5829

Merged

Conversation

ammareltigani
Copy link
Contributor

This PR starts the implementation of the placement strategy for qubit routing http://tinyurl.com/cirq-qubit-routing. A base class AbstractInitialMapper and a trivial implementation IdentityInitialMapper are added.

@ammareltigani ammareltigani requested review from a team, vtomole and cduck as code owners August 16, 2022 16:37
@ammareltigani ammareltigani requested a review from verult August 16, 2022 16:37
@CirqBot CirqBot added the size: M 50< lines changed <250 label Aug 16, 2022
Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

One question regarding the interface of initial mapper. Should we instead expose a method that expects a circuit or a (circuti, device_graph) and returns an initial mapping for that circuit / circuit, device graph combination?

I'm thinking that the class should encapsulate the strategy to map a circuit on a device; and not necessarily hardcode exact circuit / device combination as part of it's construction.

cirq-core/cirq/transformers/routing/initial_mapper.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

Can we rebase it to master so that unrelated changes in mapping manager don't appear here? We can merge once it's rebased.

@ammareltigani ammareltigani force-pushed the routing-initial_mapping_setup branch from 9ba9cef to c899e16 Compare August 18, 2022 00:01
@tanujkhattar tanujkhattar merged commit 9e8c056 into quantumlib:master Aug 19, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…tumlib#5829)

* added abstract initial mapper and identity initial mapper

* added __str__ and __repr__ for MappingManager

* minor bug

* made MappingManager not serializable

* removed unused import

* addressed comments

* fixed bug with edges not being sorted for graph equality testing

* fixed bug with digraphs repr method in MappingManager and added test for it

* rebase

* minor lint fix

* addressed comments

* addressed some comments

* changed interface for AbstractInitialMapper

* made MappingManager serializable

* removed print statements

* ready for merging

* nit

* temp

* fix lint

* removed serialization

* removed unused imports

* addressed comments and made HardCodedInitialMapper not serializable

* fixed nit

* fixed raises docstring

* forgot to add import statement

* import bug

* removed debug print
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
…tumlib#5829)

* added abstract initial mapper and identity initial mapper

* added __str__ and __repr__ for MappingManager

* minor bug

* made MappingManager not serializable

* removed unused import

* addressed comments

* fixed bug with edges not being sorted for graph equality testing

* fixed bug with digraphs repr method in MappingManager and added test for it

* rebase

* minor lint fix

* addressed comments

* addressed some comments

* changed interface for AbstractInitialMapper

* made MappingManager serializable

* removed print statements

* ready for merging

* nit

* temp

* fix lint

* removed serialization

* removed unused imports

* addressed comments and made HardCodedInitialMapper not serializable

* fixed nit

* fixed raises docstring

* forgot to add import statement

* import bug

* removed debug print
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants