-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
interface to execute(), transpile() and assemble() #2166
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ajavadia
requested review from
1ucian0,
atilag,
chriseclectic,
delapuente,
diego-plan9,
ewinston,
jaygambetta,
kdk,
mtreinish,
nonhermitian and
taalexander
as code owners
April 22, 2019 04:25
ajavadia
force-pushed
the
transpile-interface
branch
2 times, most recently
from
April 22, 2019 15:39
3f36266
to
fc5c443
Compare
1ucian0
reviewed
Apr 22, 2019
1ucian0
reviewed
Apr 22, 2019
1ucian0
reviewed
Apr 22, 2019
1ucian0
previously approved these changes
Apr 22, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not fully sure about the changes in the API, but I dont want to block here since it looks good from the code perspective.
1ucian0
reviewed
Apr 22, 2019
ajavadia
force-pushed
the
transpile-interface
branch
2 times, most recently
from
April 25, 2019 07:19
ed2e0ae
to
2434413
Compare
ajavadia
changed the title
interface to transpile() and assemble_circuits()
interface to execute(), transpile() and assemble()
Apr 25, 2019
1ucian0
reviewed
Apr 25, 2019
ajavadia
force-pushed
the
transpile-interface
branch
from
April 26, 2019 01:30
2434413
to
964c07b
Compare
jaygambetta
approved these changes
Apr 26, 2019
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
* update transpile docstring * parse transpile options from kwarg or transpile_config or backend * refactor transpiler for better encapsulation * adapt execute and assembler * adapt tools.compiler and circuits_to_qobj and some tests * fix last test: KeyError not TranspilerError * rebase and clean up * add assemble() to support both circuit and pulse assembling * define TranspileConfig and RunConfig models * revert the TranspileConfig/RunConfig models for now * cleanup assembler, pass run_config to assemble_circuits and assemble_schedules * remove assembler checks and warnings * bring back pass_manager arg in transpile for parallizability * move transpiler test to test.python.compiler
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This is a refactoring of
qiskit.compiler
to define the user interfaces and the internal method interfaces.There are 3 main user-facing functions:
transpile()
,assemble()
, andexecute()
.All 3 can handle both circuits and pulse schedules. All have an expansive set of arguments, which are overloaded heavily in certain cases for convenience (e.g. an initial_layout can be passed as a dict, or list, or Layout object).
A parsing stage separates this from the internal methods and data-structures.
Internally we have well-defined methods (e.g.
transpile_circuit()
takes aQuantumCircuit
and aTranspileConfig
. The TranspileConfig has a model defined whereby it only allows initial_layout to be of type Layout, etc. etc.)Details
Deprecates and moves the
transpile()
function from theqiskit.transpiler
namespace to theqiskit.compiler
namespace, which is where all transpilation, scheduling and assembling functions will reside.transpile()
(and thereforeexecute()
) functions now accept a list for all kwargs, similar to how they accept a list of circuits. This is so each circuit's transpilation can be configured independently, yet all run together. (Fixes Initial_layout success should not depend on QuantumRegister name #1945, Fixes transpile should be imported in * #2178, Fixes execute ignores basis_gates argument #2190. Closes need function to get TranspileConfig for a backend #1953 by making it moot.)Internally, Qiskit parses and creates a TranspileConfig from these, and then selects an appropriate PassManager based on that TranspileConfig (Closes need function to get TranspileConfig for a backend #1953 by making it moot).
Extensive documentation for the
transpile()
,assemble
andexecute()
functions.Several modes of input are now supported to the
assemble_circuits()
function, including via individual kwargs (e.g. shots), or from a RunConfig. Refer to documentation for more details.execute()
is now a thin wrapper (3 lines) aroundtranspile()
,assemble()
,backend.run()
seed_mapper
is deprecated --> useseed_transpiler
seed
is deprecated --> useseed_simulator
Follow-ups
I have made TODO comments for these:
The TranspileConfig and RunConfig models have to be formalized based on Marshmallow. I did this in but I reverted it due to a few bugs.
The
optimization_level
field of transpile_config has to be tied to choosing a pass manager. I'll do this next.The
transpile_circuit
function is a bit awkward now as it accepts a tuple of(circuit, transpile_config)
. This was so that it could be parallelized. In general we should make theparallel_map
function be able to handle multi-variable parallelizations (essentially like a zip).