Rework handling of instruction durations in preset pass managers #12183
Conversation
…on to individual passes.
|
One or more of the the following people are requested to review this:
|
| @@ -51,6 +51,7 @@ def __init__(self, inst_durations: InstructionDurations = None, target: Target = | |||
| self.inst_durations = inst_durations or InstructionDurations() | |||
| if target is not None: | |||
| self.inst_durations = target.durations() | |||
| self._durations_provided = inst_durations is not None or target is not None | |||
There was a problem hiding this comment.
I had to add this extra variable because unlike the other passes, this instantiates an empty InstructionDurations.
Pull Request Test Coverage Report for Build 8875014693Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| @@ -0,0 +1,11 @@ | |||
| --- | |||
| features_transpiler: | |||
There was a problem hiding this comment.
Is this more of a fix than a feature? At a high level this change doesn't feel like a user facing feature to document on the release. Or are you thinking that for using the individual passes in isolation this is a new feature that you can have the pass handle the durations in the circuit on their own?
There was a problem hiding this comment.
I can see that, I found the change somewhere in between a feature and a bugfix, it does change the behavior of the individual passes but probably to the behavior that would have been expected anyways, so I'm fine changing it to a fix.
| @@ -453,7 +409,7 @@ def callback_func(**kwargs): | |||
| optimization_method=optimization_method, | |||
| _skip_target=_skip_target, | |||
| ) | |||
| out_circuits = pm.run(circuits, callback=callback) | |||
| out_circuits = pm.run(circuits, callback=callback, num_processes=num_processes) | |||
There was a problem hiding this comment.
Do you think we should have a fix release note for this? This argument totally wouldn't ever work before, we were setting the flag in the single process path and we weren't setting the argument when the call could be multiprocess.
ElePT
added
the
Changelog: Bugfix
Summary
This PR fixes #10449 by removing the "hack" in
transpilethat handled the priority order of different sources ofInstructionDurationsin scheduling passes. The passes now analyze the dag calibrations and set the relevant value themselves, which allows for a much easier use of the standalone passes.I believe that the change in line 412 (where
num_processeswasn't used) was a bug that also got fixed in this PR, unless I am missing something.Details and comments
Connected to #9256.