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

Leverage Python SharedMemory for common transpile args #7789

Merged
merged 27 commits into from
Jun 22, 2022

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Mar 17, 2022

Summary

This commit uses the Python shared memory library to reduce the overhead
of launching parallel processes as part of transpile. As the size of the
backends grow the payload size we're serializing and copying between
worker processes is also increasing. When we're running a lot of small
circuits at once on a big backend we can easily spend far more time
dealing with IO overhead than running the transpilation. By using shared
memory this reduces the overhead to only serializing and copying it once
and then each worker process just needs to serializing it. While this
doesn't remove all the overhead it should reduce the impact somewhat.

Details and comments

Partially Addresses: #7741

TODO:

  • Fix failing tests
  • Cleanup code
  • Only use shared memory if we're going to run in parallel (the overhead is high for serial execution)

This commit uses the Python shared memory library to reduce the overhead
of launching parallel processes as part of transpile. As the size of the
backends grow the payload size we're serializing and copying between
worker processes is also increasing. When we're running a lot of small
circuits at once on a big backend we can easily spend far more time
dealing with IO overhead than running the transpilation. By using shared
memory this reduces the overhead to only serializing and copying it once
and then each worker process just needs to serializing it. While this
doesn't remove all the overhead it should reduce the impact somewhat.

Fixes Qiskit#7741
@mtreinish mtreinish added on hold Can not fix yet performance labels Mar 17, 2022
@mtreinish mtreinish requested a review from a team as a code owner March 17, 2022 20:50
@mtreinish
Copy link
Member Author

mtreinish commented Mar 17, 2022

This still needs a bit of work, there are a couple of edge cases with some of the transpiler args that I still need to figure out where the serialization is going wrong (or where I messed up the handling of them).

But doing some quick testing with this PR locally by running:

import time

from qiskit import QuantumCircuit, transpile
from qiskit.test.mock import FakeWashington

backend = FakeWashington()
circs = [QuantumCircuit() for _ in range(127)]
start = time.perf_counter()
transpile(circs, backend)
stop = time.perf_counter()
print(stop - start)

Is showing very promising numbers.

With this PR the test script returns:

35.77090725500602 sec

Without this PR it returns:

159.20562417100882 sec

Without this PR and setting QISKIT_PARALLEL=FALSE it returns:

4.954779412015341 sec

This PR in parallel is still slower than without this PR and QISKIT_PARALLEL=FALSE, but it's significant improvement when running in parallel. There might be some other tuning of the parallel args we can do to speed it up a bit more, but at a certain point because the worker processes are separate processes we will have overhead associated with the IPC.

@nonhermitian
Copy link
Contributor

For some reason I get this on my Linux box:

~/Downloads/qiskit-core-shm-transpile/qiskit/compiler/transpiler.py in transpile(circuits, backend, basis_gates, inst_map, coupling_map, backend_properties, initial_layout, layout_method, routing_method, translation_method, scheduling_method, instruction_durations, dt, approximation_degree, timing_constraints, seed_transpiler, optimization_level, callback, output_name, unitary_synthesis_method, unitary_synthesis_plugin_config, target)
    315                 cmap_conf = [shared_args["coupling_map"]] * len(circuits)
    316             _check_circuits_coupling_map(circuits, cmap_conf, backend)
--> 317             pickle.dump(shared_args, buf)
    318             data = buf.getvalue()
    319         smb = smm.SharedMemory(size=len(data))

~/mambaforge/envs/qiskit/lib/python3.8/site-packages/symengine/lib/symengine_wrapper.cpython-38-x86_64-linux-gnu.so in symengine.lib.symengine_wrapper.Symbol.__reduce_cython__()

TypeError: self.thisptr cannot be converted to a Python object for pickling

@nonhermitian
Copy link
Contributor

Ahh symengine 0.8.1 is not your friend here, had to go to 0.9.2.

@nonhermitian
Copy link
Contributor

With this PR I am seeing ~100sec on the same machine that ran 330sec in the original issue #7741

@mtreinish mtreinish requested a review from jyu00 as a code owner March 23, 2022 22:40
The serialization overhead of using shared memory is only necessary when
we're running in parallel. If we're running serially there is no need to
serialize the transpiler arguments and write them to shared memory since
serially we are running the same memory space. This commit splits the
code path between serial and parallel and only uses shared memory when
we are running in parallel.
@mtreinish
Copy link
Member Author

This is failing tests because of a pre-existing bug in the test. Prior to this PR the transpile() function was incorrectly handling the basis_gates and is not converting the list of basis_gates ['u', 'cx', 'measure'] to [['u', 'cx', 'measure']] as expected and causes the UnitarySynthesis pass to not be run which is masking the bug in the pass causing the failure. This PR fixes that issue part of the refactor so we're running UnitarySynthesis and are getting hit by the bug.

The move to only having one copy of shared arguments fixed an issue in
the handling of basis gates in some cases where a list input was not
properly wrapped in a list. This was then causing UnitarySynthesis pass
to be skipped in some cases because the target basis was incomplete (and
was only a single gate) which wouldn't match any of the basis the
synthesis pass could work in. By fixing this issue with shared memory
a bug in UnitarySynthesis was exposed when trying to run the pass with
no coupling map but a Target (i.e. an ideal simulator) the pass would
fail because it assumed if there was a Target there was a coupling map
and would try to use the coupling map to find the natural direction to
synthesize the 2q gates into. This commit fixes the unitary synthesis
pass to ignore the natural direction if no coupling map is preset even
if there is a target. The test triggering this condition is then updated
to update the result to include the output of the unitary synthesis
pass.
@mtreinish mtreinish changed the title [WIP] Leverage Python SharedMemory for common transpile args Leverage Python SharedMemory for common transpile args Jun 3, 2022
mtreinish and others added 6 commits June 7, 2022 08:43
This commit relaxes the output circuit equality check on the optimization
level 3 variant of the test_target_ideal_gates() test. Since this branch
fixes the basis gate handling so that the UnitarySynthesis pass is
actually running now the exact output is dependent on the 2q synthesis
routine which on different platforms has subtlely different but equally
correct results based on differences in the the floating point
implementation. To avoid spurious CI failures this commit changes the
test just check the output unitary of the circuit is equivalent to the
input for optimization level 3.
In CI we were seeing failures on the macOS Python 3.7 environment around
writing the pickle bytes data to the memory view of the shared memory
object that we allocated. This failure was being caused by the macOS
shared memory backport library over allocating the shared memory object
and the size of the memory view buffer being larger than the pickle
data. This caused the assignment of the pickle data to the memory view
to fail because the lengths didn't match. This commit fixes this by
explicitly only writing the bytes equal to the pickle length into
memory. This ensures no matter how big the allocated memory view is we
are always writing the correct size.

Co-Authored-By: Jake Lishman <[email protected]>
This commit removes a leftover from earlier iterations of the PR that
were manually manipulation of QISKIT_IN_PARALLEL. This was done earlier
in the PR branch because parallel_map was manually replaced with
multiprocessing directly. However, after refining the use of shared
memory this wasn't needed and parallel_map was used instead.
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Mostly minor comments. Do you have the latest timings for overhead with this iteration of the PR?

qiskit/compiler/transpiler.py Outdated Show resolved Hide resolved
qiskit/compiler/transpiler.py Outdated Show resolved Hide resolved
qiskit/compiler/transpiler.py Show resolved Hide resolved
qiskit/compiler/transpiler.py Outdated Show resolved Hide resolved
@mtreinish
Copy link
Member Author

Mostly minor comments. Do you have the latest timings for overhead with this iteration of the PR?

I'll run some numbers now, last time I checked it was mostly unchanged from earlier iterations

@mtreinish
Copy link
Member Author

Running the script from: #7789 (comment) takes:

~37 seconds

for me locally with the current state of the PR. This is down from the ~160 seconds I was getting without this PR. With QISKIT_PARALLEL=FALSE that script takes:

~5.5 seconds

@mtreinish mtreinish removed the Changelog: None Do not include in changelog label Jun 21, 2022
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Looks good to me. This is definitely a big improvement on parallel processing time, even if the current transpile API doesn't quite let us get all the way.

@jakelishman jakelishman added Changelog: Bugfix Include in the "Fixed" section of the changelog automerge labels Jun 22, 2022
@mergify mergify bot merged commit dbc81a8 into Qiskit:main Jun 22, 2022
@mtreinish mtreinish deleted the shm-transpile branch March 30, 2023 22:15
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Jun 15, 2023
This commit updates the transpile() function to no longer support
broadcast of lists of arguments. This functionality was deprecated in
the 0.23.0 release. As part of this removal the internals of the
transpile() function are simplified so we don't need to handle
broadcasting, building preset pass managers, parallel dispatch, etc
anymore as this functionality (without broadcasting) already exists
through the transpiler API. Besides greatly simplifying the transpile()
code and using more aspects of the public APIs that exist in the
qiskit.transpiler module, this commit also should fix the overhead we
have around parallel execution due to the complexity of supporting
broadcasting. This overhead was partially addressed before in Qiskit#7789
which leveraged shared memory to minimize the serialization time
necessary for IPC but by using `PassManager.run()` internally now all of
that overhead is removed as the initial fork will have all the necessary
context in each process from the start.

Three seemingly unrelated changes made here were necessary to support our
current transpile() API without building custom pass manager
construction.

The first is the handling of layout from intlist. The
current Layout class is dependent on a circuit because it maps Qubit
objects to a physical qubit index. Ideally the layout structure would
just map virtual indices to physical indices (see Qiskit#8060 for a similar
issue, also it's worth noting this is how the internal NLayout and QPY
represent layout), but because of the existing API the construction of
a Layout is dependent on a circuit. For the initial_layout argument when
running with multiple circuits to avoid the need to broadcasting the
layout construction for supported input types that need the circuit to
lookup the Qubit objects the SetLayout pass now supports taking in an
int list and will construct a Layout object at run time. This
effectively defers the Layout object creation for initial_layout to
run time so it can be built as a function of the circuit as the API
demands.

The second is the FakeBackend class used in some tests was constructing
invalid backends in some cases. This wasn't caught in the previous
structure because the backends were not actually being parsed by
transpile() previously which masked this issue. This commit fixes that
issue because PassManagerConfig.from_backend() was failing because of
the invalid backend construction.

The third issue is a new _skip_target private argument to
generate_preset_pass_manager() and PassManagerConfig. This was necessary
to recreate the behavior of transpile() when a user provides a BackendV2
and either `basis_gates` or `coupling_map` arguments. In general the
internals of the transpiler treat a target as higher priority because it
has more complete and restrictive constraints than the
basis_gates/coupling map objects. However, for transpile() if a
backendv2 is passed in for backend paired with coupling_map and/or
basis_gates the expected workflow is that the basis_gates and
coupling_map arguments take priority and override the equivalent
attributes from the backend. To facilitate this we need to block pulling
the target from the backend This should only be needed for a short
period of time as when Qiskit#9256 is implemented we'll just build a single
target from the arguments as needed.

Fixes Qiskit#7741
github-merge-queue bot pushed a commit that referenced this pull request Jul 19, 2023
* Remove list argument broadcasting and simplify transpile()

This commit updates the transpile() function to no longer support
broadcast of lists of arguments. This functionality was deprecated in
the 0.23.0 release. As part of this removal the internals of the
transpile() function are simplified so we don't need to handle
broadcasting, building preset pass managers, parallel dispatch, etc
anymore as this functionality (without broadcasting) already exists
through the transpiler API. Besides greatly simplifying the transpile()
code and using more aspects of the public APIs that exist in the
qiskit.transpiler module, this commit also should fix the overhead we
have around parallel execution due to the complexity of supporting
broadcasting. This overhead was partially addressed before in #7789
which leveraged shared memory to minimize the serialization time
necessary for IPC but by using `PassManager.run()` internally now all of
that overhead is removed as the initial fork will have all the necessary
context in each process from the start.

Three seemingly unrelated changes made here were necessary to support our
current transpile() API without building custom pass manager
construction.

The first is the handling of layout from intlist. The
current Layout class is dependent on a circuit because it maps Qubit
objects to a physical qubit index. Ideally the layout structure would
just map virtual indices to physical indices (see #8060 for a similar
issue, also it's worth noting this is how the internal NLayout and QPY
represent layout), but because of the existing API the construction of
a Layout is dependent on a circuit. For the initial_layout argument when
running with multiple circuits to avoid the need to broadcasting the
layout construction for supported input types that need the circuit to
lookup the Qubit objects the SetLayout pass now supports taking in an
int list and will construct a Layout object at run time. This
effectively defers the Layout object creation for initial_layout to
run time so it can be built as a function of the circuit as the API
demands.

The second is the FakeBackend class used in some tests was constructing
invalid backends in some cases. This wasn't caught in the previous
structure because the backends were not actually being parsed by
transpile() previously which masked this issue. This commit fixes that
issue because PassManagerConfig.from_backend() was failing because of
the invalid backend construction.

The third issue is a new _skip_target private argument to
generate_preset_pass_manager() and PassManagerConfig. This was necessary
to recreate the behavior of transpile() when a user provides a BackendV2
and either `basis_gates` or `coupling_map` arguments. In general the
internals of the transpiler treat a target as higher priority because it
has more complete and restrictive constraints than the
basis_gates/coupling map objects. However, for transpile() if a
backendv2 is passed in for backend paired with coupling_map and/or
basis_gates the expected workflow is that the basis_gates and
coupling_map arguments take priority and override the equivalent
attributes from the backend. To facilitate this we need to block pulling
the target from the backend This should only be needed for a short
period of time as when #9256 is implemented we'll just build a single
target from the arguments as needed.

Fixes #7741

* Fix _skip_target logic

* Fix InstructionScheduleMap handling with backendv2

* Fix test failure caused by exception being raised later

* Fix indentation error

* Update qiskit/providers/fake_provider/fake_backend.py

Co-authored-by: John Lapeyre <[email protected]>

* Fix standalone dt argument handling

* Remove unused code

* Fix lint

* Remove duplicate import in set_layout.py

A duplicate import slipped through in the most recent rebase.
This commit fixes that oversight and removes the duplicate.

* Update release notes

Co-authored-by: Jake Lishman <[email protected]>

* Adjust logic for _skip_transpile to check if None

* Simplify check cmap code

* Only check backend if it exists

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
to24toro pushed a commit to to24toro/qiskit-terra that referenced this pull request Aug 3, 2023
)

* Remove list argument broadcasting and simplify transpile()

This commit updates the transpile() function to no longer support
broadcast of lists of arguments. This functionality was deprecated in
the 0.23.0 release. As part of this removal the internals of the
transpile() function are simplified so we don't need to handle
broadcasting, building preset pass managers, parallel dispatch, etc
anymore as this functionality (without broadcasting) already exists
through the transpiler API. Besides greatly simplifying the transpile()
code and using more aspects of the public APIs that exist in the
qiskit.transpiler module, this commit also should fix the overhead we
have around parallel execution due to the complexity of supporting
broadcasting. This overhead was partially addressed before in Qiskit#7789
which leveraged shared memory to minimize the serialization time
necessary for IPC but by using `PassManager.run()` internally now all of
that overhead is removed as the initial fork will have all the necessary
context in each process from the start.

Three seemingly unrelated changes made here were necessary to support our
current transpile() API without building custom pass manager
construction.

The first is the handling of layout from intlist. The
current Layout class is dependent on a circuit because it maps Qubit
objects to a physical qubit index. Ideally the layout structure would
just map virtual indices to physical indices (see Qiskit#8060 for a similar
issue, also it's worth noting this is how the internal NLayout and QPY
represent layout), but because of the existing API the construction of
a Layout is dependent on a circuit. For the initial_layout argument when
running with multiple circuits to avoid the need to broadcasting the
layout construction for supported input types that need the circuit to
lookup the Qubit objects the SetLayout pass now supports taking in an
int list and will construct a Layout object at run time. This
effectively defers the Layout object creation for initial_layout to
run time so it can be built as a function of the circuit as the API
demands.

The second is the FakeBackend class used in some tests was constructing
invalid backends in some cases. This wasn't caught in the previous
structure because the backends were not actually being parsed by
transpile() previously which masked this issue. This commit fixes that
issue because PassManagerConfig.from_backend() was failing because of
the invalid backend construction.

The third issue is a new _skip_target private argument to
generate_preset_pass_manager() and PassManagerConfig. This was necessary
to recreate the behavior of transpile() when a user provides a BackendV2
and either `basis_gates` or `coupling_map` arguments. In general the
internals of the transpiler treat a target as higher priority because it
has more complete and restrictive constraints than the
basis_gates/coupling map objects. However, for transpile() if a
backendv2 is passed in for backend paired with coupling_map and/or
basis_gates the expected workflow is that the basis_gates and
coupling_map arguments take priority and override the equivalent
attributes from the backend. To facilitate this we need to block pulling
the target from the backend This should only be needed for a short
period of time as when Qiskit#9256 is implemented we'll just build a single
target from the arguments as needed.

Fixes Qiskit#7741

* Fix _skip_target logic

* Fix InstructionScheduleMap handling with backendv2

* Fix test failure caused by exception being raised later

* Fix indentation error

* Update qiskit/providers/fake_provider/fake_backend.py

Co-authored-by: John Lapeyre <[email protected]>

* Fix standalone dt argument handling

* Remove unused code

* Fix lint

* Remove duplicate import in set_layout.py

A duplicate import slipped through in the most recent rebase.
This commit fixes that oversight and removes the duplicate.

* Update release notes

Co-authored-by: Jake Lishman <[email protected]>

* Adjust logic for _skip_transpile to check if None

* Simplify check cmap code

* Only check backend if it exists

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
ElePT pushed a commit to ElePT/qiskit-ibm-provider that referenced this pull request Oct 4, 2023
…skit#10291)

* Remove list argument broadcasting and simplify transpile()

This commit updates the transpile() function to no longer support
broadcast of lists of arguments. This functionality was deprecated in
the 0.23.0 release. As part of this removal the internals of the
transpile() function are simplified so we don't need to handle
broadcasting, building preset pass managers, parallel dispatch, etc
anymore as this functionality (without broadcasting) already exists
through the transpiler API. Besides greatly simplifying the transpile()
code and using more aspects of the public APIs that exist in the
qiskit.transpiler module, this commit also should fix the overhead we
have around parallel execution due to the complexity of supporting
broadcasting. This overhead was partially addressed before in Qiskit/qiskit#7789
which leveraged shared memory to minimize the serialization time
necessary for IPC but by using `PassManager.run()` internally now all of
that overhead is removed as the initial fork will have all the necessary
context in each process from the start.

Three seemingly unrelated changes made here were necessary to support our
current transpile() API without building custom pass manager
construction.

The first is the handling of layout from intlist. The
current Layout class is dependent on a circuit because it maps Qubit
objects to a physical qubit index. Ideally the layout structure would
just map virtual indices to physical indices (see Qiskit/qiskit#8060 for a similar
issue, also it's worth noting this is how the internal NLayout and QPY
represent layout), but because of the existing API the construction of
a Layout is dependent on a circuit. For the initial_layout argument when
running with multiple circuits to avoid the need to broadcasting the
layout construction for supported input types that need the circuit to
lookup the Qubit objects the SetLayout pass now supports taking in an
int list and will construct a Layout object at run time. This
effectively defers the Layout object creation for initial_layout to
run time so it can be built as a function of the circuit as the API
demands.

The second is the FakeBackend class used in some tests was constructing
invalid backends in some cases. This wasn't caught in the previous
structure because the backends were not actually being parsed by
transpile() previously which masked this issue. This commit fixes that
issue because PassManagerConfig.from_backend() was failing because of
the invalid backend construction.

The third issue is a new _skip_target private argument to
generate_preset_pass_manager() and PassManagerConfig. This was necessary
to recreate the behavior of transpile() when a user provides a BackendV2
and either `basis_gates` or `coupling_map` arguments. In general the
internals of the transpiler treat a target as higher priority because it
has more complete and restrictive constraints than the
basis_gates/coupling map objects. However, for transpile() if a
backendv2 is passed in for backend paired with coupling_map and/or
basis_gates the expected workflow is that the basis_gates and
coupling_map arguments take priority and override the equivalent
attributes from the backend. To facilitate this we need to block pulling
the target from the backend This should only be needed for a short
period of time as when Qiskit/qiskit#9256 is implemented we'll just build a single
target from the arguments as needed.

Fixes Qiskit/qiskit#7741

* Fix _skip_target logic

* Fix InstructionScheduleMap handling with backendv2

* Fix test failure caused by exception being raised later

* Fix indentation error

* Update qiskit/providers/fake_provider/fake_backend.py

Co-authored-by: John Lapeyre <[email protected]>

* Fix standalone dt argument handling

* Remove unused code

* Fix lint

* Remove duplicate import in set_layout.py

A duplicate import slipped through in the most recent rebase.
This commit fixes that oversight and removes the duplicate.

* Update release notes

Co-authored-by: Jake Lishman <[email protected]>

* Adjust logic for _skip_transpile to check if None

* Simplify check cmap code

* Only check backend if it exists

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Oct 10, 2023
…skit#10291)

* Remove list argument broadcasting and simplify transpile()

This commit updates the transpile() function to no longer support
broadcast of lists of arguments. This functionality was deprecated in
the 0.23.0 release. As part of this removal the internals of the
transpile() function are simplified so we don't need to handle
broadcasting, building preset pass managers, parallel dispatch, etc
anymore as this functionality (without broadcasting) already exists
through the transpiler API. Besides greatly simplifying the transpile()
code and using more aspects of the public APIs that exist in the
qiskit.transpiler module, this commit also should fix the overhead we
have around parallel execution due to the complexity of supporting
broadcasting. This overhead was partially addressed before in Qiskit/qiskit#7789
which leveraged shared memory to minimize the serialization time
necessary for IPC but by using `PassManager.run()` internally now all of
that overhead is removed as the initial fork will have all the necessary
context in each process from the start.

Three seemingly unrelated changes made here were necessary to support our
current transpile() API without building custom pass manager
construction.

The first is the handling of layout from intlist. The
current Layout class is dependent on a circuit because it maps Qubit
objects to a physical qubit index. Ideally the layout structure would
just map virtual indices to physical indices (see Qiskit/qiskit#8060 for a similar
issue, also it's worth noting this is how the internal NLayout and QPY
represent layout), but because of the existing API the construction of
a Layout is dependent on a circuit. For the initial_layout argument when
running with multiple circuits to avoid the need to broadcasting the
layout construction for supported input types that need the circuit to
lookup the Qubit objects the SetLayout pass now supports taking in an
int list and will construct a Layout object at run time. This
effectively defers the Layout object creation for initial_layout to
run time so it can be built as a function of the circuit as the API
demands.

The second is the FakeBackend class used in some tests was constructing
invalid backends in some cases. This wasn't caught in the previous
structure because the backends were not actually being parsed by
transpile() previously which masked this issue. This commit fixes that
issue because PassManagerConfig.from_backend() was failing because of
the invalid backend construction.

The third issue is a new _skip_target private argument to
generate_preset_pass_manager() and PassManagerConfig. This was necessary
to recreate the behavior of transpile() when a user provides a BackendV2
and either `basis_gates` or `coupling_map` arguments. In general the
internals of the transpiler treat a target as higher priority because it
has more complete and restrictive constraints than the
basis_gates/coupling map objects. However, for transpile() if a
backendv2 is passed in for backend paired with coupling_map and/or
basis_gates the expected workflow is that the basis_gates and
coupling_map arguments take priority and override the equivalent
attributes from the backend. To facilitate this we need to block pulling
the target from the backend This should only be needed for a short
period of time as when Qiskit/qiskit#9256 is implemented we'll just build a single
target from the arguments as needed.

Fixes Qiskit/qiskit#7741

* Fix _skip_target logic

* Fix InstructionScheduleMap handling with backendv2

* Fix test failure caused by exception being raised later

* Fix indentation error

* Update qiskit/providers/fake_provider/fake_backend.py

Co-authored-by: John Lapeyre <[email protected]>

* Fix standalone dt argument handling

* Remove unused code

* Fix lint

* Remove duplicate import in set_layout.py

A duplicate import slipped through in the most recent rebase.
This commit fixes that oversight and removes the duplicate.

* Update release notes

Co-authored-by: Jake Lishman <[email protected]>

* Adjust logic for _skip_transpile to check if None

* Simplify check cmap code

* Only check backend if it exists

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Oct 12, 2023
)

* Remove list argument broadcasting and simplify transpile()

This commit updates the transpile() function to no longer support
broadcast of lists of arguments. This functionality was deprecated in
the 0.23.0 release. As part of this removal the internals of the
transpile() function are simplified so we don't need to handle
broadcasting, building preset pass managers, parallel dispatch, etc
anymore as this functionality (without broadcasting) already exists
through the transpiler API. Besides greatly simplifying the transpile()
code and using more aspects of the public APIs that exist in the
qiskit.transpiler module, this commit also should fix the overhead we
have around parallel execution due to the complexity of supporting
broadcasting. This overhead was partially addressed before in Qiskit#7789
which leveraged shared memory to minimize the serialization time
necessary for IPC but by using `PassManager.run()` internally now all of
that overhead is removed as the initial fork will have all the necessary
context in each process from the start.

Three seemingly unrelated changes made here were necessary to support our
current transpile() API without building custom pass manager
construction.

The first is the handling of layout from intlist. The
current Layout class is dependent on a circuit because it maps Qubit
objects to a physical qubit index. Ideally the layout structure would
just map virtual indices to physical indices (see Qiskit#8060 for a similar
issue, also it's worth noting this is how the internal NLayout and QPY
represent layout), but because of the existing API the construction of
a Layout is dependent on a circuit. For the initial_layout argument when
running with multiple circuits to avoid the need to broadcasting the
layout construction for supported input types that need the circuit to
lookup the Qubit objects the SetLayout pass now supports taking in an
int list and will construct a Layout object at run time. This
effectively defers the Layout object creation for initial_layout to
run time so it can be built as a function of the circuit as the API
demands.

The second is the FakeBackend class used in some tests was constructing
invalid backends in some cases. This wasn't caught in the previous
structure because the backends were not actually being parsed by
transpile() previously which masked this issue. This commit fixes that
issue because PassManagerConfig.from_backend() was failing because of
the invalid backend construction.

The third issue is a new _skip_target private argument to
generate_preset_pass_manager() and PassManagerConfig. This was necessary
to recreate the behavior of transpile() when a user provides a BackendV2
and either `basis_gates` or `coupling_map` arguments. In general the
internals of the transpiler treat a target as higher priority because it
has more complete and restrictive constraints than the
basis_gates/coupling map objects. However, for transpile() if a
backendv2 is passed in for backend paired with coupling_map and/or
basis_gates the expected workflow is that the basis_gates and
coupling_map arguments take priority and override the equivalent
attributes from the backend. To facilitate this we need to block pulling
the target from the backend This should only be needed for a short
period of time as when Qiskit#9256 is implemented we'll just build a single
target from the arguments as needed.

Fixes Qiskit#7741

* Fix _skip_target logic

* Fix InstructionScheduleMap handling with backendv2

* Fix test failure caused by exception being raised later

* Fix indentation error

* Update qiskit/providers/fake_provider/fake_backend.py

Co-authored-by: John Lapeyre <[email protected]>

* Fix standalone dt argument handling

* Remove unused code

* Fix lint

* Remove duplicate import in set_layout.py

A duplicate import slipped through in the most recent rebase.
This commit fixes that oversight and removes the duplicate.

* Update release notes

Co-authored-by: Jake Lishman <[email protected]>

* Adjust logic for _skip_transpile to check if None

* Simplify check cmap code

* Only check backend if it exists

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Dec 8, 2023
…skit#10291)

* Remove list argument broadcasting and simplify transpile()

This commit updates the transpile() function to no longer support
broadcast of lists of arguments. This functionality was deprecated in
the 0.23.0 release. As part of this removal the internals of the
transpile() function are simplified so we don't need to handle
broadcasting, building preset pass managers, parallel dispatch, etc
anymore as this functionality (without broadcasting) already exists
through the transpiler API. Besides greatly simplifying the transpile()
code and using more aspects of the public APIs that exist in the
qiskit.transpiler module, this commit also should fix the overhead we
have around parallel execution due to the complexity of supporting
broadcasting. This overhead was partially addressed before in Qiskit/qiskit#7789
which leveraged shared memory to minimize the serialization time
necessary for IPC but by using `PassManager.run()` internally now all of
that overhead is removed as the initial fork will have all the necessary
context in each process from the start.

Three seemingly unrelated changes made here were necessary to support our
current transpile() API without building custom pass manager
construction.

The first is the handling of layout from intlist. The
current Layout class is dependent on a circuit because it maps Qubit
objects to a physical qubit index. Ideally the layout structure would
just map virtual indices to physical indices (see Qiskit/qiskit#8060 for a similar
issue, also it's worth noting this is how the internal NLayout and QPY
represent layout), but because of the existing API the construction of
a Layout is dependent on a circuit. For the initial_layout argument when
running with multiple circuits to avoid the need to broadcasting the
layout construction for supported input types that need the circuit to
lookup the Qubit objects the SetLayout pass now supports taking in an
int list and will construct a Layout object at run time. This
effectively defers the Layout object creation for initial_layout to
run time so it can be built as a function of the circuit as the API
demands.

The second is the FakeBackend class used in some tests was constructing
invalid backends in some cases. This wasn't caught in the previous
structure because the backends were not actually being parsed by
transpile() previously which masked this issue. This commit fixes that
issue because PassManagerConfig.from_backend() was failing because of
the invalid backend construction.

The third issue is a new _skip_target private argument to
generate_preset_pass_manager() and PassManagerConfig. This was necessary
to recreate the behavior of transpile() when a user provides a BackendV2
and either `basis_gates` or `coupling_map` arguments. In general the
internals of the transpiler treat a target as higher priority because it
has more complete and restrictive constraints than the
basis_gates/coupling map objects. However, for transpile() if a
backendv2 is passed in for backend paired with coupling_map and/or
basis_gates the expected workflow is that the basis_gates and
coupling_map arguments take priority and override the equivalent
attributes from the backend. To facilitate this we need to block pulling
the target from the backend This should only be needed for a short
period of time as when Qiskit/qiskit#9256 is implemented we'll just build a single
target from the arguments as needed.

Fixes Qiskit/qiskit#7741

* Fix _skip_target logic

* Fix InstructionScheduleMap handling with backendv2

* Fix test failure caused by exception being raised later

* Fix indentation error

* Update qiskit/providers/fake_provider/fake_backend.py

Co-authored-by: John Lapeyre <[email protected]>

* Fix standalone dt argument handling

* Remove unused code

* Fix lint

* Remove duplicate import in set_layout.py

A duplicate import slipped through in the most recent rebase.
This commit fixes that oversight and removes the duplicate.

* Update release notes

Co-authored-by: Jake Lishman <[email protected]>

* Adjust logic for _skip_transpile to check if None

* Simplify check cmap code

* Only check backend if it exists

---------

Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog Changelog: Bugfix Include in the "Fixed" section of the changelog performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants