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

Fix error in schedule -> signal conversion for schedules with barrier instructions #203

Merged
merged 3 commits into from
Mar 14, 2023

Conversation

DanPuzzuoli
Copy link
Collaborator

Summary

Closes #202.

This fixes an error in the schedule -> signal converter when the schedule contained a barrier instruction.

Details and comments

As described in #202, in the loop over instructions to construct the signals, there is a line chan = inst.channel.name, which assumes that inst.channel exists. Barrier instructions do not have a channel attribute, hence the error. This PR modifies this line to conditionally set chan based on whether inst.channel exists, and adds a test validating the correct parsing of a schedule with a barrier. Note that I've also removed the phase and freq variables set at the same time as chan, as they were only used in one subsequent line.

For completeness, the important thing to note here is that nothing actually needs to be done to "support" barrier instructions:

  • Barrier instructions impact how block_to_schedule converts ScheduleBlock instances to Schedule instances.
  • Once in Schedule form all instructions have explicit absolute timing, so the barrier instructions in the schedule are actually redundant.
  • As the Dynamics schedule -> signal converter works with these absolute times, it can actually just ignore the barrier instructions when parsing a schedule.
  • Hence, the problem arising in Add support for barriers in schedule -> signal converter #202 is just a result of a trivial mishandling of the barrier instruction object.

ihincks
ihincks previously approved these changes Mar 14, 2023
Copy link
Contributor

@ihincks ihincks left a comment

Choose a reason for hiding this comment

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

LGTM, optional suggestion included.

qiskit_dynamics/pulse/pulse_to_signals.py Show resolved Hide resolved
@DanPuzzuoli DanPuzzuoli merged commit 27cf27c into qiskit-community:main Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for barriers in schedule -> signal converter
2 participants