Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

Fix DynamicCircuitInstructionDurations.from_backend for both Backend versions#787

Merged
kt474 merged 17 commits into
Qiskit:mainfrom
dieris:delay-unit-conversion
Feb 7, 2024
Merged

Fix DynamicCircuitInstructionDurations.from_backend for both Backend versions#787
kt474 merged 17 commits into
Qiskit:mainfrom
dieris:delay-unit-conversion

Conversation

@dieris
Copy link
Copy Markdown
Contributor

@dieris dieris commented Jan 8, 2024

Summary

Fixes an issue preventing DynamicInstructionDurations.from_backend(backend) from constructing the corresponding object with a backend whose durations are not in units of dt. Note that prior to Qiskit/qiskit#11501, a InstructionDurations object of the parent class was created instead (with no duration patching).

Details and comments

This is meant to fix the use case for dynamic circuits e.g.

from qiskit.providers.fake_provider import FakeKolkata
from qiskit_ibm_provider.transpiler.passes.scheduling import DynamicCircuitInstructionDurations, ALAPScheduleAnalysis, PadDynamicalDecoupling

durations = DynamicCircuitInstructionDurations.from_backend(FakeKolkata())
pm = PassManager([ALAPScheduleAnalysis(durations),
                  PadDynamicalDecoupling(durations, dd_sequence)])
circ_dd = pm.run(qc)

@dieris
Copy link
Copy Markdown
Contributor Author

dieris commented Jan 8, 2024

@mbhealy could you confirm if this fix is still necessary?
@jakelishman this is the issue I was referring to in using DynamicInstructionDurations.from_backend. Is backend.target.durations() the recommended way to load durations for a BackendV2 instead?

@jakelishman
Copy link
Copy Markdown
Member

Diego: Target.durations is most likely correct, and we should probably update the InstructionDurations.from_backend classmethod to call it if it's a BackendV2, I guess. In that case, your subclass of InstructionDurations will still need to override the classmethod in order to lift the return value of Target.duration to your desired form.

@mtreinish
Copy link
Copy Markdown
Member

qiskit.target.durations is the canonical location to get an InstructionDurations object in the BackendV2 model. In general the InstructionDurations class is just a legacy/compatibility view of the information contained in the Target and really is only needed for places that have compatibility with both BackendV1 and BackendV2 and if you're only working in V2 you shouldn't need to use it because the Target class integrates the duration information as part of it's data model.

There is also backend.instruction_durations (https://docs.quantum.ibm.com/api/qiskit/qiskit.providers.BackendV2#instruction_durations) which by default just calls self.target.durations() (https://github.com/Qiskit/qiskit/blob/main/qiskit/providers/backend.py#L411-L414)

@mbhealy
Copy link
Copy Markdown
Contributor

mbhealy commented Jan 8, 2024

@mbhealy could you confirm if this fix is still necessary?

Yes, that fix is still important to include.

@dieris dieris changed the title Convert durations to dt before patching Fix DynamicCircuitInstructionDurations.from_backend for both Backend versions Jan 8, 2024
@dieris
Copy link
Copy Markdown
Contributor Author

dieris commented Jan 9, 2024

The new tests and test_transpile_both_paths can only pass with the qiskit fix linked in the description, allowing for the correct creation of a DynamicCircuitInstructionDurations object with patched durations.

Copy link
Copy Markdown
Member

@ajavadia ajavadia left a comment

Choose a reason for hiding this comment

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

this looks good. let's merge it soon so this can work with qiskit 1.0

Copy link
Copy Markdown
Contributor

@kt474 kt474 left a comment

Choose a reason for hiding this comment

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

LGTM, @dieris can you make sure these changes are also reflected in qiskit-ibm-runtime?

@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 7809604102

  • -6 of 54 (88.89%) changed or added relevant lines in 2 files are covered.
  • 9 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 43.323%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_ibm_provider/transpiler/passes/scheduling/utils.py 47 53 88.68%
Files with Coverage Reduction New Missed Lines %
qiskit_ibm_provider/transpiler/plugin.py 9 45.16%
Totals Coverage Status
Change from base Build 7805663899: 0.2%
Covered Lines: 2933
Relevant Lines: 6770

💛 - Coveralls

@kt474 kt474 merged commit 4adc2d8 into Qiskit:main Feb 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants