Improve performance of ConstrainedReschedule pass
#10077
Conversation
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
alexanderivrii
changed the title
ConstrainedReschedule passConstrainedReschedule pass
kdk
added
stable backport potential
| try: | ||
| misalignment = node_start_time[node] % alignment | ||
| if misalignment == 0: | ||
| continue | ||
| shift = max(0, alignment - misalignment) | ||
| start_time = node_start_time[node] | ||
| except KeyError as ex: | ||
| raise TranspilerError( | ||
| f"Start time of {repr(node)} is not found. This node is likely added after " | ||
| "this circuit is scheduled. Run scheduler again." | ||
| ) from ex |
There was a problem hiding this comment.
I think this pattern is much cleaner.
start_time = node_start_time.get(node)
if start_time is None:
raise TranspilerError(....
There was a problem hiding this comment.
Agreed, done in 6d21886.
| else: | ||
| # Directive or delay. These can start at arbitrary time. |
There was a problem hiding this comment.
I would be explicit about a isinstance(node.op, [Directive, Delay]) branch and leave the else branch for raising on an unknown op type, except that really means that all the non-gate/non-measure can start at arbitrary time.
There was a problem hiding this comment.
Done in 6d21886.
Except that for checking whether node.op is a directive we probably need to check getattr(node.op, "_directive", False) and not isinstance(node.op, Directive), the Directive class seems to be something relevant for pulses.
* experiments * deleting unused file * cleanup * removing duplicate implementation * clarifying docstring * applying suggestions from code review (cherry picked from commit c3d7d5a)
* experiments * deleting unused file * cleanup * removing duplicate implementation * clarifying docstring * applying suggestions from code review (cherry picked from commit c3d7d5a) Co-authored-by: Alexander Ivrii <[email protected]>
* experiments * deleting unused file * cleanup * removing duplicate implementation * clarifying docstring * applying suggestions from code review
Summary
This PR reimplements the
ConstrainedRescheduletranspiler pass to run in linear time, by going over the nodes once and pushing back the start times in case of misalignments and overlaps. This should solve the performance problem described in #10067.Details and comments
The older commit 6c5eab6 contains both the original and the modified version of
ConstrainedReschedulepass (with the newer code residing intranspiler/passes/scheduling/alignments/reschedule_improved.py), allowing to run both versions and compare the output circuits. On my laptop, for therandom_circuit(50, 5, measure=True, seed=202342), the new code runs in0.43seconds compared to the old code running in444.25seconds.I have tried to keep the code logic as close as possible to the original (in order to avoid introducing bugs), there are definitely places that could be rewritten in a more clear and shorter fashion.
@mtreinish has further suggestions to improve runtimes based on bd3cbb2.