Split scheduling pass into scheduling and padding

[nkanazawa1989]

Now functionality of scheduling pass is split into two pieces; scheduling (analysis) and padding (transform). In between these operations, one can insert non-transform operation such as alignment (i.e. reschedule).

Summary

This PR splits functionality of scheduling passes into scheduling (analysis) and padding (transform). This is beneficial since one can interleave alignment passes, i.e. rescheduling, in between these operation without regenerating DAG. In addition, scheduling operation creates time_slot dictionary in the property set of the pass manager. This allows us to easily track instruction start time without accumulating instruction duration from the begging of the DAG wire.

Details and comments

Note that this is breaking API change since now scheduling passes insert no delay. In addition, consecutive delays are now merged into a single delay, which is necessary to correctly evaluate alignment constraints (will be in follow-up PR). Because it is not easy to dynamically switch base class (analysis -> transform pass) just for backward compatibility (technically possible though), current approach is to allow this API change and to write enough information in the release note. Since scheduling pass is used only in the specific situation (e.g. noisy circuit simulation, evaluation of alignment constraints, dynamical decoupling), the impact of this change is expected to be limited.

[coveralls]

Pull Request Test Coverage Report for Build 1961014800

• 81 of 87 (93.1%) changed or added relevant lines in 11 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.003%) to 83.456%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/passes/scheduling/base_scheduler.py	6	7	85.71%
qiskit/transpiler/preset_passmanagers/level0.py	2	3	66.67%
qiskit/transpiler/preset_passmanagers/level2.py	2	3	66.67%
qiskit/transpiler/preset_passmanagers/level3.py	2	3	66.67%
qiskit/transpiler/passes/scheduling/padding.py	54	56	96.43%

Totals
Change from base Build 1960638229:	0.003%
Covered Lines:	52381
Relevant Lines:	62765

---

💛 - Coveralls

[itoko]

I agree with the direction splitting out the padding part from scheduling passes so that we can remove the overhead of creating new DAG multiple times (e.g. ASAPSchedule and AlignMeasure) in the future.

For its implementation, I think we need more discussion on what should be stored in property_set by the new ASAP/ALAPSchedule analysis pass. Currently, "start times" are stored as a dict {node: t0 (start time)}. However, the t0 information seems essentially not used in the newly implemented BasePadding pass. I'm thinking the minimal data to be stored may be "idle slots", i.e. (prev_node, node(next_node), duration), and t0 of the node is supplemental information that could be ignored in rescheduling passes such as AlignMeasure. What do you think?

I've left other minor comments and suggestions inline.

[nkanazawa1989]

Thanks @itoko! It might be hard to grasp the entire picture of the refactoring only with this PR, but align measure and align pulse are the constraints on t0. The main target of time_slot (now it's renamed to node_start_time) is indeed these alignment passes in which currently t0 are computed by integrating duration of all ancestor nodes on the wire. Thus having start time rather than idle time makes the logic much faster.

I was also thinking to store (t0, t1) tuple in the property set, but I decided to store only t0 since t1 = t0 = node.duration (having both is bit redundant).

[itoko]

Oh, I missed that. You're right. Alignment constraints are about t0 (not duration), so we need to store t0s even if it is not directly used in Padding passes. Also, not storing t1 (and prev_node as well) sounds reasonable to me.

[itoko]

Uhm, this is a tricky problem.. I'd like to hear more opinion on how to address this issue. I personally prefer to simply using DAGOpNode instance as a key here (i.e. give up to raise an error when someone updates node.op without updating node itself) and notify that users should not update DAG before using note_start_time property in docstrings.
Also, I'm not sure if the current key can always track the update of the node. For example, what happens if only the params are replaced in node.op?

[nkanazawa1989]

I understand your point. This code is indeed not robust to the future change of node data structure. I guess the mutability of node.op is due to performance of the Unroller node, which is likely intentional and a pass manager designer must take care for the arrangement of passes. In that sense, the padding pass here can also take node as-is rather than decomposing it for the key, and let the designer carefully choose the order of passes.

[itoko]

Minor comments on comments

[itoko]

Thank you for the updates, @nkanazawa1989. LGTM now.
I approve without adding automerge tag so that you could ask one more final check from other reviewers as this PR includes a big design change.

[kdk]

Thanks for taking this on @nkanazawa1989 and @itoko . The approach here LGTM.

[ajavadia]

small comments mostly on docstrings

[ajavadia]

LGTM