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

generate_preset_pass_manager should take initial_layout as a list #11690

Closed
ajavadia opened this issue Jan 31, 2024 · 3 comments · Fixed by #12214
Closed

generate_preset_pass_manager should take initial_layout as a list #11690

ajavadia opened this issue Jan 31, 2024 · 3 comments · Fixed by #12214
Assignees
Labels
type: feature request New feature or request
Milestone

Comments

@ajavadia
Copy link
Member

What should we add?

Now that #10344 is merged, we should make the generate_preset_pass_manager function take an integer list for layout. This is a user entry point and should be easy to use. The Layout object is a pain to use all around (I would love to completely eliminate it, since it doesn't contain any more information than a list of int. The circuit qubits have an order).

@ajavadia ajavadia added the type: feature request New feature or request label Jan 31, 2024
@ajavadia ajavadia added this to the 1.0.0 milestone Jan 31, 2024
@mtreinish mtreinish modified the milestones: 1.0.0, 1.1.0 Jan 31, 2024
@sbrandhsn
Copy link
Contributor

We have a discussion about refactoring the Layout class in #11604 , @ajavadia could you give your opinion there? :-)

@ajavadia
Copy link
Member Author

ajavadia commented Feb 2, 2024

I think there are some other issues with the generate_preset_pass_manager. Even when an actual Layout object is supplied, it fails with TranspilerError: 'Sabre swap runs on physical circuits only.'. Needs more exploration.

@mtreinish
Copy link
Member

@ajavadia do you have a re-create. I wrote a quick test to check this and it seems to at least work for my test:

        coupling_map_list = [[0, 1]]
            
        # Circuit that doesn't fit in the coupling map
        qc = QuantumCircuit(2)
        qc.h(0)
        qc.cx(0, 1)
        qc.cx(1, 0)
        qc.measure_all()
                        
        pm_list = generate_preset_pass_manager(
            optimization_level=optimization_level,
            coupling_map=coupling_map_list,
            basis_gates=["u", "cx"],
            seed_transpiler=42,
            initial_layout=[1, 0],
        )   
        pm_object = generate_preset_pass_manager(
            optimization_level=optimization_level,
            coupling_map=coupling_map_list,
            basis_gates=["u", "cx"],
            seed_transpiler=42,
            initial_layout=Layout.from_intlist([1, 0], *qc.qregs),
        )
        tqc_list = pm_list.run(qc)
        tqc_obj = pm_list.run(qc)
        self.assertIsInstance(pm_list, PassManager)
        self.assertIsInstance(pm_object, PassManager)
        
        self.assertEqual(tqc_list, tqc_obj)

(I checked the output circuits and they worked fine). Is the issue just the type hint for the argument?

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Apr 18, 2024
This commit updates the documentation for the
generate_preset_pass_manager() function to clearly indicate that the
function will accept a integer list for the initial_layout field. This
already was supported but it wasn't documented. As it's now a documented
part of the API a unittest is added to ensure we don't regress this
functionality in the future.

Fixes Qiskit#11690
github-merge-queue bot pushed a commit that referenced this issue Apr 18, 2024
#12214)

This commit updates the documentation for the
generate_preset_pass_manager() function to clearly indicate that the
function will accept a integer list for the initial_layout field. This
already was supported but it wasn't documented. As it's now a documented
part of the API a unittest is added to ensure we don't regress this
functionality in the future.

Fixes #11690
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants