Skip registration of U gates in Default setting in pulse system

[to24toro]

What should we add?

Previously, IBM provider used to offer U gates u1, u2, and u3 as basis gates. Now, the target class includes information about backend instructions, properties and more. Additionally, the backend default is also included in the target and u gates are allowed to be registered to the default for the backward compatibility in

qiskit/qiskit/providers/models/pulsedefaults.py

Line 205 in cd507ad

for inst in cmd_def:

.

for inst in cmd_def:
  entry = PulseQobjDef(converter=self.converter, name=inst.name)
  entry.define(inst.sequence, user_provided=False)
  self.instruction_schedule_map._add(
    instruction_name=inst.name,
   qubits=tuple(inst.qubits),
   entry=entry,
   )

When creating the backend default, instruction names and theire qubit arguments are mapped to the schedule through instruction_schedule_map, resulting in the inclusion of U gate instructions in the target.
We can exclude U gates from inst_map by skipping registering u gates.

for inst in cmd_def:
  # add new codes that skip registration of U gates
  if inst.name in {"u1", "u2", "u3"}:
    continue
  entry = PulseQobjDef(converter=self.converter, name=inst.name)
  entry.define(inst.sequence, user_provided=False)
  self.instruction_schedule_map._add(
    instruction_name=inst.name,
   qubits=tuple(inst.qubits),
   entry=entry,
   )

[nkanazawa1989]

You need to add more background for discussion. I had related discussion with @mtreinish but maybe it was offline and I don't remember the details.

Currently you are updating circuit scheduler with #10564, and the legacy ScheduleConfig object consisting of inst map and meas map will be replaced with Target. This means, for backward compatibility, Target must gain a capability of creating an instance with user provided inst map and meas map, which is not the supported case of Target.from_configuration because it doesn't create new instruction entry with a schedule in the inst map. So we decided to update Target.update_from_instruction_schedule_map method instead.

This will allow circuit-pulse scheduler to use Target, instead of the legacy config object.

# Create an empty Target
target = Target(meas_map=schedule_config.meas_map, dt=schedule_config.dt)
target.update_from_instruction_schedule_map(schedule_config.inst_map)

Problems of using the update_from_instruction_schedule_map method are:

• This method is not designed to fill empty Target object (i.e. it is designed to apply user-provided diffs to Target)
• This method is not aware of backend configuration
• IBM backends still report U1, U2, U3 in the inst map (which is coming from CmdDef qobj).

Current implementation of the method is (pseudo code):

for all schedules in inst_map object:
    if schedule is backend provided and its name is not in Target:
        ignore schedule
    else:
        do create Qiskit gate object repr from schedule name
        do create InstructionProperties and add it to Target

This means, necessary gates such as SX and RZ are also ignored because they don't exist in the empty Target that we want to deal with. We should relax this condition to copy them from the inst map, but this also allow U1, U2, U3 schedules to show up in the created Target. This will create an edge case; if a user reuses this Target object for compilation, circuit might be accidentally decomposed with U gates and fail in backend execution.

We should properly handle three situations:

Backend provided schedule, NOT in configuration.supported_instructions and configuration.basis_gates

This is the case of U1, U2, U3. Entries are not reported in circuit domain thus not in the created Target.
These schedules can be accessed through the legacy attribute backend.defaults().instruction_schedule_map.

Backend provided, defined in configuration.supported_instructions or configuration.basis_gates

This is the case of SX, RZ. Entries are created in the Target with calibration information.
User can get schedules from the Target.

User provided

Custom defined gate (not in basis gates) or basis gates with updated calibration.
This schedule must be registered in the Target because the entry is intentionally added by the user.

In the current implementation we cannot distinguish (1) and (2) only with the inst_map (i.e. without backend configuration), which is the situation of converting the legacy ScheduleConfig into Target.

Roughly speaking, I think there are two approaches:

(1) Permanently removing U1, U2, U3 from inst map.
(2) Make update_from_instruction_schedule_map method basis gate aware, and let users provide basis gates.

The approach you are proposing is one of the implementations for (1). I think there are several patterns to realize (1) and (2). Can you summarize the implementation and write pros/cons for each? @to24toro

[to24toro]

(1) Permanently removing U1, U2, U3 from inst map.

If we can skip registering these U gate such as U1, U2, U3 in backend.defaults().instruction_schedule_map, we can prevent their inclusion in the Target during the update_from_instruction_schedule_map process.

Pros:

1. Simplification of Target objects because it eliminates the possibility of inadvertently registering the U gates to the Target and consequently compiling circuit that includes the Target registering U gates.
2. Eliminates confusion regarding the presence of U gates when working with the Target in the future.

Cons:

1. It represents a significant change to the API of backend.default() as it involves skipping the registration of U gates solely for the purpose of updating the Target. Currently, the case of creating Target from empty and updating it is only used in circuit scheduling with backendV2. While the actual code change may be minor, there could be unforeseen impacts that we are currently unaware of.

(2) Make update_from_instruction_schedule_map method basis gate aware, and let users provide basis gates.

Pros:

1. Users gain the flexibility to choose which basis gates they want tot include in the Target and explicitly recognize the basis gates used.
2. Changes are localized the code of update_from_instruction_schedule_map rather than backend.default(). This change would have a minimal impact compared to the previous proposal of (1).

Cons:

1. It increases the number of arguments for update_from_instruction_schedule_map by making it basis gate aware. Users would need to manually specify which basis gates to include when creating a Target from scratch, especially in the case of circuit scheduling. But this can be avoided by setting configuration.basis_gate as the basis gate if users don't specify the basis gates. For most people, they wouldn't intentionally use the U gate.

[mtreinish]

TBH, I don't have a super strong opinion on this. I'm still confused why the backend is returning calibrations for U1, U2, and U3 at all if they're not supported operations. This does seem like something that is backend specific, because if the InstructionScheduleMap that backend.defaults().instruction_schedule_map is returning is not representative of what the backend actually is using. I guess what I'm saying is that feels more like a custom concern for specific IBM backends and the ideal solution for me would be to update https://github.com/Qiskit/qiskit-ibm-provider/blob/main/qiskit_ibm_provider/utils/json_decoder.py#L43 to exclude u1, u2, u3 in the generation of the PulseDefaults object if they're not needed/used anymore.

That all being said, I'm not actually too concerned with adding a basis_gates or use_instructions option to Target.update_from_instruction_schedule_map, yes it's another option on the method but I can see a general use case if you only wanted to partial update a Target from an InstructionScheduleMap for like one or two gates. It does put the burden on users to know how to do this though, and that might be a big ask. Maybe instead of an inclusion list what about an exclusion list, so Target.update_from_instruction_schedule_map(..., ignore_instruction=['u1', 'u2', 'u3')). Maybe adding both options to let users selectively filter how they update the target from an instruction schedule map is the best path forward here.