[WIP] Solve a bug of scheduling with backendV2 #9912

to24toro · 2023-04-05T05:33:53Z

Summary

Based on the comment,
I added a new class MeasureGrouping. It is assumed to be called by target.meas_map.
Using this class, we can become to create scheduling for quantum circuits dynamically.

This PR includes

adding a new class MeasureGrouping.
dispatching macros.measure.

Details and comments

fix #9488
Solving this issue leads to working on #9420.

[TODO]

implement get_qubit_groups in MeasureGrouping.
implement generate_schedule which yield something corresponding to inst_schedule_map.
call MeasureGrouping when create Target class in backendV2.
add add_measuregrouping to qiskit_ibm_provider/backend_converter.py

qiskit-bot · 2023-04-05T05:33:56Z

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:

@Qiskit/terra-core
@nkanazawa1989

TsafrirA · 2023-04-09T09:45:08Z

qiskit/pulse/macros.py

+        if backend is None or hasattr(backend, "target"):
+            if target is None:
+                target = backend.target


I think the logic here is problematic.
If you provide as arguments inst_map and meas_map, the macro has all the information it needs, but the condition here will raise the exception.

…p_class

nkanazawa1989

Thanks @to24toro this is great start of V2 migration of the pulse module. I also want review from @TsafrirA .

nkanazawa1989 · 2023-04-13T02:08:30Z

qiskit/pulse/macros.py

+            "inst_map or meas_map, and backend cannot be None simultaneously"
+        ) from ex
+    if isinstance(meas_map, list):
+        meas_map = utils.format_meas_map(meas_map)


Can you integrate this into MeasureGrouping?

This function is used in only _measure_v1. So I think that it is no necessary to move into MeasureGrouping.

qiskit/pulse/macros.py

nkanazawa1989 · 2023-04-13T02:10:34Z

qiskit/pulse/macros.py

+def measure_v2(
+    qubits: List[int],
+    backend=None,
+    target: Optional[Target] = None,


You can reduce backend and target by formatting the arguments in measure function before dispatching the logic. Please keep interface minimum for maintainability. Same for v1.

nkanazawa1989 · 2023-04-13T02:11:29Z

qiskit/pulse/macros.py

+        measure_groups.add(tuple(meas_map[qubit]))
+    for measure_group_qubits in measure_groups:
+        try:
+            default_sched = target.get_calibration(measure_name, measure_group_qubits)


This doesn't work because Target has calibration for single qubit (because attached Measure class is single qubit operation). Here, you need to iterate over all qubits in the group and combine them.

Also, I have been wondering if grouping is really needed without constraints. For example, if you combine meas calibration for group of [0, 1] and [2, 3], this will eventually generate a left-aligned block of [0, 1, 2, 3]. I think we just need to combine them without considering the grouping, as long as backend meas map doesn't provide constraint. Probably, we need to pad non-measured qubit timeslot in the group with delays (so that they don't cause overlap within the group). Any thoughts @wshanks ?

nkanazawa1989 · 2023-04-13T02:25:43Z

qiskit/pulse/macros.py

@@ -104,3 +189,21 @@ def measure_all(backend) -> Schedule:
        A schedule corresponding to the inputs provided.
    """
    return measure(qubits=list(range(backend.configuration().n_qubits)), backend=backend)
+
+
+def combine_schedules(qubits: List, schedule: Schedule, default_schedule: Union[Schedule, ScheduleBlock],measure_group_qubits: List, qubit_mem_slots: Optional[Dict[int, int]] = None):


I think this should be a function remap_memory_slot(schedule: Schedule, qubit_mem_slots: Dict[int, int]) -> Schedule because it seems just replacing the slot index. @TsafrirA does it help creg index matching? Let us know if you have other ideas.

I change the code at 509392a

nkanazawa1989 · 2023-04-13T02:31:42Z

qiskit/pulse/macros.py

+    if hasattr(backend, "target"):
+        return measure_v2(qubits, backend, target, meas_map, qubit_mem_slots, measure_name)


Suggested change

if hasattr(backend, "target"):

return measure_v2(qubits, backend, target, meas_map, qubit_mem_slots, measure_name)

if target or hasattr(backend, "target"):

return measure_v2(

qubits=qubits,

target=target or backend.target,

meas_map=meas_map or getattr(backend, "meas_map", MeasureGrouping()),

qubit_mem_slots=qubit_mem_slots,

measure_name=measure_name,

)

Note that backend is optional here.

I change the code at 509392a

nkanazawa1989 · 2023-04-13T02:34:00Z

qiskit/transpiler/target.py

@@ -1045,6 +1048,10 @@ def build_coupling_map(self, two_q_gate=None):
        else:
            return None

+    def add_measuregrouping(self, meas_map=None):


You can update the constructor of Target with meas_map (because this is hardware constraints and user cannot modify once it's set). Then store it as a protected member and provide only property method for it so that user cannot override.

nkanazawa1989 · 2023-04-13T02:36:30Z

qiskit/pulse/macros.py

@@ -16,12 +16,14 @@

 from qiskit.pulse import channels, exceptions, instructions, utils
 from qiskit.pulse.instruction_schedule_map import InstructionScheduleMap
-from qiskit.pulse.schedule import Schedule
+from qiskit.pulse.schedule import Schedule, ScheduleBlock
+from qiskit.transpiler.target import Target


 def measure(


Can you start pending deprecation for inst_map and V1 backend? We have a convenient decorator now:
https://github.com/Qiskit/qiskit-terra/blob/58aceb59e9bc1e16f29b09402e1984b16d211da3/qiskit/utils/deprecation.py#L100-L111

Should I move deprecate_arg part to _measure_v1?

nkanazawa1989 · 2023-04-13T02:41:30Z

qiskit/transpiler/measuregrouping.py

+        """
+        Gets qubit groups including at least one qubit of `qubits` from meas_map.
+        """
+        return [


For backward compatibility, you need to add case for self.meas_map = None. I think, in this case, you just need to return input qubits as-is (because no grouping constraint is applied).

to24toro · 2023-04-18T02:37:33Z

qiskit/pulse/macros.py

+from qiskit.transpiler.target import Target
+from qiskit.utils.deprecation import deprecate_arg


It seems that this import statement causes a circular import, so we need to resolve it.

to24toro · 2023-04-18T02:50:41Z

qiskit/pulse/macros.py

+    if meas_map is not None:
+        if meas_map is Dict:
+            pass
+        else:
+            pass
+
+    meas_map = target.meas_map.get_qubit_groups(qubits)


In _measure_v2, meas_map is assumed to be a List. For example, the conventional meas_map = [[1, 2], [2, 3, 4]] will be converted to [1, 2, 3, 4].
If meas_map is None as input, you can obtain a List like [1, 2, 3, 4] from meas_map = target.meas_map.get_qubit_groups(qubits).

However, processing for Dict and List[List] types of meas_map has not been implemented yet.
I would like to know what would be the best way to handle Dict and List[List] types of meas_map.

to24toro · 2023-04-18T02:56:17Z

qiskit/transpiler/target.py

@@ -315,6 +319,7 @@ def __init__(
                        "length of the input qubit_properties list"
                    )
        self.qubit_properties = qubit_properties
+        self._meas_group = MeasureGrouping(meas_group)


I defined MeasureGrouping in meas_group. I named as meas_group because meas_map exists in MeasureGrouping. I would like to know if the naming is reasonable.

macros.py target delete raise statement in measure_v2 modify instructions.Acquire to Acquire

to24toro · 2023-05-12T06:28:04Z

I would like to re-submit a PR to organize it again. See #10105.

to24toro added 3 commits April 5, 2023 10:44

create measuregrouping class

4bab6a6

add meas_map.setter in MeasureGrouping class

9d109d0

macros.measure

a40211c

nkanazawa1989 self-assigned this Apr 5, 2023

nkanazawa1989 added the mod: pulse Related to the Pulse module label Apr 5, 2023

TsafrirA reviewed Apr 9, 2023

View reviewed changes

to24toro added 7 commits April 10, 2023 17:54

get_qubit_groups

25d0719

generate_schedule

11f734b

target.add_measuregrouping in backend_compat

ca3f032

target.add_measuregrouping in backend_converter

2d2ff79

reformat and add docs

0d4b715

Merge remote-tracking branch 'origin/main' into feature/add_meas_grou…

d223f4f

…p_class

on the way of working on generate_schedule_in_measure

d29025f

nkanazawa1989 requested changes Apr 13, 2023

View reviewed changes

to24toro force-pushed the feature/add_meas_group_class branch 3 times, most recently from 5da678c to 509392a Compare April 18, 2023 02:28

to24toro commented Apr 18, 2023

View reviewed changes

to24toro force-pushed the feature/add_meas_group_class branch 2 times, most recently from f89279e to 5977d0f Compare April 18, 2023 04:33

to24toro added 3 commits April 18, 2023 20:29

split measure into measure_v1 and measure_v2

68ec213

macros.py target delete raise statement in measure_v2 modify instructions.Acquire to Acquire

macros.py

d19464b

test_measuregrouping

5930369

to24toro force-pushed the feature/add_meas_group_class branch from 5977d0f to 5930369 Compare April 18, 2023 11:54

to24toro mentioned this pull request May 12, 2023

Add MeasureGrouping class #10105

Closed

to24toro closed this May 12, 2023

chuanqixu mentioned this pull request Jun 16, 2023

Another Potential Solution to Schedule with V2 Fails #10298

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Solve a bug of scheduling with backendV2 #9912

[WIP] Solve a bug of scheduling with backendV2 #9912

to24toro commented Apr 5, 2023 •

edited

Loading

qiskit-bot commented Apr 5, 2023

TsafrirA Apr 9, 2023

nkanazawa1989 left a comment

nkanazawa1989 Apr 13, 2023

to24toro Apr 18, 2023

nkanazawa1989 Apr 13, 2023

nkanazawa1989 Apr 13, 2023

nkanazawa1989 Apr 13, 2023

to24toro Apr 18, 2023

nkanazawa1989 Apr 13, 2023 •

edited

Loading

to24toro Apr 18, 2023

nkanazawa1989 Apr 13, 2023

nkanazawa1989 Apr 13, 2023

to24toro Apr 18, 2023

nkanazawa1989 Apr 13, 2023

to24toro Apr 18, 2023

to24toro Apr 18, 2023

to24toro Apr 18, 2023

to24toro commented May 12, 2023

		if hasattr(backend, "target"):
		return measure_v2(qubits, backend, target, meas_map, qubit_mem_slots, measure_name)

-    if hasattr(backend, "target"):
-        return measure_v2(qubits, backend, target, meas_map, qubit_mem_slots, measure_name)
+    if target or hasattr(backend, "target"):
+        return measure_v2(
+            qubits=qubits,
+            target=target or backend.target,
+            meas_map=meas_map or getattr(backend, "meas_map", MeasureGrouping()),
+            qubit_mem_slots=qubit_mem_slots,
+            measure_name=measure_name,
+        )

		from qiskit.transpiler.target import Target
		from qiskit.utils.deprecation import deprecate_arg

[WIP] Solve a bug of scheduling with backendV2 #9912

[WIP] Solve a bug of scheduling with backendV2 #9912

Conversation

to24toro commented Apr 5, 2023 • edited Loading

Summary

Details and comments

qiskit-bot commented Apr 5, 2023

Choose a reason for hiding this comment

nkanazawa1989 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkanazawa1989 Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

to24toro commented May 12, 2023

to24toro commented Apr 5, 2023 •

edited

Loading

nkanazawa1989 Apr 13, 2023 •

edited

Loading