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

OpenPulse frontend - assembler module #2115

Merged
merged 77 commits into from
Apr 22, 2019

Conversation

nkanazawa1989
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 commented Apr 11, 2019

Summary

This is the final piece of #1713, built on #2108. This convert pulse schedule to qobj and execute it on the backend.

Details and comments

execute_schedule() internally calls assemble_schedule() to create PulseQobj and execute this on the given backend.

opts = {
    'shots': rabi_shots,
    'meas_level': 1,
    'meas_return': 'avg',
    'memory_slot_size': 1
}

job = execute_schedules(schedules, backend, user_lo_dict, **opts)
  • We still need to discuss the interface of LOs for multiple schedules and frequency sweep. Adding restriction to the length of user_lo_dict could be the best solution so far #41

  • We can find the mismatch between execute interfaces of pulse and circuit.

  • We should provide schedule validator to verify at least meas_map (I'm writing this in another PR)

@itoko
Copy link
Contributor

itoko commented Apr 12, 2019

Example codes:

[Case - No LO settings]
# compose schedule
schedule = pulse.Schedule()
#... compose schedule ...
execute_schedules(schedule, backend, **opts)
[Case - 1 schedule : n lo_configs]
# compose schedule
schedule = pulse.Schedule()
#... compose schedule ...

# set LO frequency
lo_configs=[]
for drive_lo_freq in drive_lo_freqs:
    freq_d0 = freq_d0_default + drive_lo_freq
    lo_configs.append(UseLoDict({device.q[0].drive: freq_d0}))

execute_schedules(schedule, backend, lo_configs, **opts)
[Case - n schedules : 1 lo_config]
# compose schedules
schedules = []
for ii in pulses:
    #... compose ii-th schedule ...
    schedules.append(schedule)

# set LO frequency
lo_config = UseLoDict({device.q[0].drive: freq_d0})

execute_schedules(schedules, backend, lo_config, **opts)
[Case - n*m schedules : n*m lo_configs from n schedules * m lo_configs]
# compose n schedules
schedules = []
for ii in pulses:
    #... compose ii-th schedule ...
    schedules.append(schedule)

# m LO frequency settings
lo_configs=[]
for drive_lo_freq in drive_lo_freqs:
    freq_d0 = freq_d0_default + drive_lo_freq
    lo_configs.append(UseLoDict({device.q[0].drive: freq_d0}))

exp_schedules, exp_lo_configs = [], []
for sched in schedules:
    for los in lo_configs:
       exp_schedules.append(sched)
       exp_lo_configs.append(los)
assert(len(exp_schedules) == len(exp_lo_configs))

execute_schedules(exp_schedules, backend, exp_lo_configs, **opts)

qiskit/compiler/assembler.py Outdated Show resolved Hide resolved
qiskit/execute.py Outdated Show resolved Hide resolved
Copy link
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

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

This is coming along @nkanazawa1989 and @itoko but I think requires some major revisions with respect to schedule configuration.

Also, how should we update execute.execute to account for schedules? Thoughts @ajavadia?

qiskit/compiler/assembler.py Outdated Show resolved Hide resolved

def __call__(self, instruction):

for name, method in inspect.getmembers(self, inspect.ismethod):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this just be registered with a class attribute upon the application of bind_instruction, rather than performing introspection of the class attributes?

qiskit/compiler/pulse_to_qobj.py Outdated Show resolved Hide resolved
qiskit/compiler/pulse_to_qobj.py Outdated Show resolved Hide resolved
qiskit/compiler/assembler.py Outdated Show resolved Hide resolved
qiskit/pulse/channels/output_channel.py Outdated Show resolved Hide resolved
qiskit/pulse/channels/output_channel.py Outdated Show resolved Hide resolved
qiskit/pulse/schedule/conditioned_schedule.py Outdated Show resolved Hide resolved
qiskit/pulse/schedule/experiment_condition.py Outdated Show resolved Hide resolved
from qiskit.pulse.exceptions import PulseError


class UserLoDict:
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is required, should it be a more general ExpConfig or something along these lines?

@taalexander
Copy link
Contributor

fyi, the above review was intended to be of Request Changes, but I misclicked.

@nkanazawa1989
Copy link
Contributor Author

Thank you @taalexander ! I added some major changes for lo related stuff with another PR #44. I think it makes the thing more simpler.

ConditionedSchedule comes from @itoko. Of course we can pass UserLoConfig to assembler without making ConditionedSchedule. I think it is introduced to explicitly bind schedule and LO configuration.

@taalexander
Copy link
Contributor

I updated the PR to remove run_config and move to passing keyword arguments as suggested by @ajavadia offline.

Copy link
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.

can you make sure the signatures of assemble_schdules and execute_schedules are grouped in the same order? for example qobj_header appears before schedule_los in one and after in the other.
The two functions should have almost identical signatures (except obvious things like execute takes a backend). After all execute is supposed to be a simpler wrapper around making a schedule, then assembling it and running it. Can you make sure there's good reason if their signatures diverge in any place?

qiskit/execute.py Outdated Show resolved Hide resolved
qiskit/execute.py Show resolved Hide resolved
qiskit/execute.py Outdated Show resolved Hide resolved
qiskit/compiler/assembler.py Outdated Show resolved Hide resolved
qiskit/execute.py Outdated Show resolved Hide resolved
qiskit/execute.py Outdated Show resolved Hide resolved
@ajavadia ajavadia merged commit 08965f6 into Qiskit:master Apr 22, 2019
@taalexander taalexander mentioned this pull request Apr 22, 2019
@nkanazawa1989 nkanazawa1989 deleted the openpulse-assembler-pr branch May 13, 2019 01:17
lia-approves pushed a commit to edasgupta/qiskit-terra that referenced this pull request Jul 30, 2019
* add assembler for schedule (@itoko, @nkanazawa1989)

* update tests

* ensure value is complex

* convert list to tuple before taking lo_range from backend

* add execute assembler to init

* lint

* switch back to original spec about #schedule:#user_lo_dicts

* remove default pulse lib from qobj config

* lint

* fix bug when pulse_library is not defined

* Update assembler.py

docstring fix

* Update assembler.py

docstring update.

* Fix docstrings.

* add pulse backend mock

* rename lo_frequency to lo_freq

* get freq_est from backend.defaults()

* update test

* remove conditioned_schedule

* remove conditioned_schedule

* fix bugs

* update assembler test

* lint

* restructure

* optimize import

* remove empty kernel and discriminator

* update test

* add lo_range check __eq__

* update UserLoDict to check channel type

* remove _replaced_with_user_los

* move converter under qobj

* update test

* add lo converter

* add lo converter

* update docstring

* Change LoDict to LoConfig.

* removed lo_converter as argument and hardcoded instead. This should reduce the complexity of the interface for now. We can always readd.

* Assembler now accepts lo_config as a dictionary (which is then converted to an LoConfig).

* Remove unnecessary return placeholders and commented print statements from tests.

* Renamed LoDictConverter to LoConfigConverter.

* linting

* import linting

* Update LO kwarg documentation

* Remove continuation statements by wrapping with parenthesis.

* Remove lo range and lofreq from output channel identity check.

* Remove dependence of channel hash on frequency.

* Remove lo freq from channel hash.

* Fix test assembler docstrings.

* Remove unnecessary placeholder assignment.

* Restructured assemble_schedules to remove run_config and pass arguments as kwargs.

* renamed lo_configs to schedule_los.

* Change meas_lo_dict to actually return dict.

* Fix test.

* linting.

* Updated changelog.

* remove unnecessary hash.

* Removed __eq__ from output channel.

* Add to changelog to force test rerun.

* Remove unneeded raises in docstring.

* remove extra schedule los docstring.

* Fix overrwritten scedule config.

* Add memory/meas_level error messages and handling to schedules.

* Update qobj header docstring.

* Added qobjheader support for dict argument.

* Updated schedule execute and assemble signatures to match. Updated case of meas_level 2 and meas_return, warnings.
@1ucian0 1ucian0 added the mod: pulse Related to the Pulse module label Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: pulse Related to the Pulse module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants