-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
OpenPulse frontend - assembler module #2115
Conversation
Example codes:
|
There was a problem hiding this 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/pulse_to_qobj.py
Outdated
|
||
def __call__(self, instruction): | ||
|
||
for name, method in inspect.getmembers(self, inspect.ismethod): |
There was a problem hiding this comment.
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?
from qiskit.pulse.exceptions import PulseError | ||
|
||
|
||
class UserLoDict: |
There was a problem hiding this comment.
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?
fyi, the above review was intended to be of Request Changes, but I misclicked. |
Thank you @taalexander ! I added some major changes for lo related stuff with another PR #44. I think it makes the thing more simpler.
|
…a into openpulse-assembler-pr
I updated the PR to remove |
…a into openpulse-assembler-pr
There was a problem hiding this 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?
…se of meas_level 2 and meas_return, warnings.
* 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.
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 callsassemble_schedule()
to createPulseQobj
and execute this on the given backend.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 #41We 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)