Skip to content

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, cduck and vtomole 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.

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