Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pulse compiler skeleton #11743

Merged

Conversation

nkanazawa1989
Copy link
Contributor

Summary

This PR introduces a skeleton of pulse compiler based on the BasePassManager.

Details and comments

Pulse compiler is expected to support multiple output format. The format can be set at run time, i.e. an argument of .run. As in the circuit transpiler, the pulse compiler has two base passes for transform and analysis. We expect that a system configuration is provided by Qiskit Target model. In future we may integrate workflow of circuit and pulse pass manager via the ConversionPass (namely scheduler). Because all circuit passes will gain the target object in their constructor, making the Target the single source of truth for quantum program compilation would simplifies our future compiler framework.

Note that pulse compiler passes must explicitly implement __eq__ method, otherwise two different objects with the same pass configuration are evaluated as different objects (because equivalence check is deleted to object where only object id is evaluated). Two passes with the same setup must behave identically, and they must be the same object from PulseIR's point of view. Target object doesn't need to be evaluated, because it doesn't implement __eq__ and we expect that the target object is not mutated during the single compilation run -- this information is identical to all running passes.

@nkanazawa1989 nkanazawa1989 added mod: pulse Related to the Pulse module experimental Experimental feature without API stability guarantee labels Feb 7, 2024
@nkanazawa1989 nkanazawa1989 linked an issue Feb 7, 2024 that may be closed by this pull request
9 tasks
@nkanazawa1989 nkanazawa1989 mentioned this pull request Feb 7, 2024
9 tasks
Copy link
Collaborator

@TsafrirA TsafrirA 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 great @nkanazawa1989!
I left some minor comments for your consideration.

qiskit/pulse/compiler/basepasses.py Show resolved Hide resolved
qiskit/pulse/compiler/basepasses.py Show resolved Hide resolved

def __init__(
self,
target: Target,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not all passes would require Target. Do we want to force this anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is what circuit passes are trying to realize. This allows passes to reference system configuration without diverging the interface. Probably making it Optional is reasonable suggestion though.

qiskit/pulse/compiler/passmanager.py Outdated Show resolved Hide resolved
@nkanazawa1989 nkanazawa1989 marked this pull request as ready for review February 21, 2024 16:23
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core
  • @nkanazawa1989

Copy link
Collaborator

@TsafrirA TsafrirA left a comment

Choose a reason for hiding this comment

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

Looks good!

def run(
self,
schedules: ScheduleBlock | list[ScheduleBlock],
output: str = "schedule_block",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if ScheduleBlock should be the default here, as it doesn't support timing data. It's not critical though, and because there is some uncertainty about the output format, we don't need to waste time on it now.

callback: Callable | None = None,
num_processes: int | None = None,
) -> Any:
"""Run all the passes on the input pulse schedules.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The term "schedules" here (and onward) could be misleading because we're aiming for ScheduleBlock and not Schedule.

Copy link
Collaborator

@TsafrirA TsafrirA 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.
I think BlockTranspiler is a strange beast which won't see much use, because the ScheduleBlock class doesn't support information which is typically required (mostly, timing). I can imagine going from ScheduleBlock to ScheduleBlock (converting old model to new model, analyzing, etc.) but I think this mostly highlights that we are missing a front end representation - the legacy Schedule? A new format?

In any case, there is nothing wrong with BlockTranspiler, and we can move ahead, but it highlights what more needs to be done.

@nkanazawa1989
Copy link
Contributor Author

ScheduleBlock could do pseudo scheduling by flattening all instructions into left justified context with proper delay. This is how we currently convert backend calibrations in the Schedule format into ScheduleBlock. This is just for backward compatibility because we don't plan to support PulseQobj or QPy as a compiler backend; i.e.

  1. channel model conversion -> legacy transform (ScheduleBlock to Schedule) -> assemble (Schedule to PulseQobj)

  2. channel model conversion -> QPy (ScheduleBlock -> binary) -> IBM Provider

I think ScheduleBlock output is also convenient for the unit test of model conversion.

@nkanazawa1989 nkanazawa1989 merged commit af12e64 into Qiskit:feature/pulse-ir Mar 5, 2024
10 checks passed
@TsafrirA TsafrirA mentioned this pull request Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Experimental feature without API stability guarantee mod: pulse Related to the Pulse module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pulse Compiler and IR
3 participants