Skip to content

no full scheduling of circuits with delay unless explicitly requested#6084

Merged
mergify[bot] merged 12 commits into
Qiskit:masterfrom
ajavadia:fix-issue-5962
Mar 31, 2021
Merged

no full scheduling of circuits with delay unless explicitly requested#6084
mergify[bot] merged 12 commits into
Qiskit:masterfrom
ajavadia:fix-issue-5962

Conversation

@ajavadia
Copy link
Copy Markdown
Member

This is an alternative to #5972:

  • Removes the logic for implicit scheduling when a single delay was encountered.
  • It solve the original problem in Output transpiled circuit in dt #5563 by using a pass TimeUnitConversion to make units of duration consistent (either all dt if dt is known, or all s). This pass is a small modification on the already-existing TimeUnitAnalysis.
  • The ALAP and ASAP schedulers are simpler now. They just receive a circuit whose ops have been annotated with duration metadata and all units are consistent. So the schedulers can focus on the logic of scheduling only. (Hence the ALAPSchedule and ASPASchedule now require the TimeUnitConversion to meet their precondition).

Fixes #5962

@ajavadia ajavadia requested a review from a team as a code owner March 25, 2021 15:54
@ajavadia ajavadia added this to the 0.17 milestone Mar 25, 2021
@ajavadia ajavadia force-pushed the fix-issue-5962 branch 3 times, most recently from 8479569 to 34e7b0d Compare March 26, 2021 02:50
Copy link
Copy Markdown
Contributor

@itoko itoko left a comment

Choose a reason for hiding this comment

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

Thank you for taking this over. Overall, looks good to me.
I leave several comments for improvement inline.

Comment thread qiskit/transpiler/passes/scheduling/alap.py Outdated
Comment thread qiskit/transpiler/passes/scheduling/time_unit_conversion.py Outdated
Comment thread test/python/circuit/test_scheduled_circuit.py Outdated
Comment thread qiskit/transpiler/passes/scheduling/time_unit_conversion.py Outdated
@ajavadia ajavadia added the Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. label Mar 30, 2021
Copy link
Copy Markdown
Contributor

@itoko itoko left a comment

Choose a reason for hiding this comment

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

I've double checked and found two more lines using Bit.index. Sorry for not finding them earlier. Other than that, LGTM. I’ll approve after they are fixed.

Comment thread qiskit/transpiler/passes/scheduling/alap.py Outdated
Comment thread qiskit/transpiler/passes/scheduling/asap.py Outdated
information, or explicitly supply ``instruction_durations`` as an argument
to the transpiler.
instructions, the units will be converted to dt if the value of dt
(sample time) is known to the transpile (either via explicit dt arg or via
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
(sample time) is known to the transpile (either via explicit dt arg or via
(sample time) is known to the transpiler (either via explicit dt arg or via

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've made mental note of this, I'll just fix it in #6075

Copy link
Copy Markdown
Contributor

@lcapelluto lcapelluto left a comment

Choose a reason for hiding this comment

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

One typo in release notes. LGTM

@mergify mergify Bot merged commit 0dfe057 into Qiskit:master Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A crash when using delay instruction with non-dt units

5 participants