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 bug where lo units were not being properly converted #3597

Merged
merged 15 commits into from
Jan 6, 2020

Conversation

taalexander
Copy link
Contributor

Summary

Closes #3595, #3596. Fixes a bug where lo units are not being properly converted, nor were they being accepted in Hz by assemble/execute and schedule_los.

Details and comments

@taalexander taalexander added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Dec 11, 2019
@mtreinish mtreinish added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Dec 12, 2019
@@ -325,13 +325,20 @@ def __init__(self,
self.n_uchannels = n_uchannels
self.u_channel_lo = u_channel_lo
self.meas_levels = meas_levels
self.qubit_lo_range = qubit_lo_range
self.meas_lo_range = meas_lo_range
self.qubit_lo_range = [[range[0] * 1e9, range[1] * 1e9] for range in qubit_lo_range]
Copy link
Contributor

Choose a reason for hiding this comment

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

it's a little confusing seeing range as a variable and not the built in range method

Copy link
Contributor Author

@taalexander taalexander Dec 16, 2019

Choose a reason for hiding this comment

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

agreed, should be different name

Copy link
Member

@ajavadia ajavadia Dec 24, 2019

Choose a reason for hiding this comment

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

@taalexander so can you fix it. also lines 329 and 339. otherwise LGTM.

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, I will return to this soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 555f727

@lcapelluto
Copy link
Contributor

With this PR, assemble and execute will take frequency in terms of Hz, and output a qobj still in GHz, correct?

Copy link
Contributor Author

@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.

yes

@taalexander
Copy link
Contributor Author

This should be good to go now.

@@ -77,24 +77,24 @@ def assemble(experiments,
Random seed to control sampling, for when backend is a simulator

qubit_lo_freq (list):
List of default qubit LO frequencies. Will be overridden by
List of default qubit lo frequencies in Hz. Will be overridden by
Copy link
Contributor

Choose a reason for hiding this comment

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

Why lowercase?

taalexander and others added 3 commits January 6, 2020 11:11
Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>
Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>
@mergify mergify bot merged commit a187d83 into Qiskit:master Jan 6, 2020
mtreinish pushed a commit to mtreinish/qiskit-core that referenced this pull request Jan 7, 2020
* Fix backend config where lo ranges were not adjusted to Hz.

* Fix bug where assemble/execute accepted parameters in GHz rather than Hz.

* linting

* Documentation fixes for Hz.

* Add channel bandwidth.

* bug fix with kwarg pop.

* Missing None in kwargs pop.

* linting.

* Fix range in list comprehension for lo ranges in backend configuration.

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* lo to LO in docstrings.

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Lauren Capelluto <laurencapelluto@gmail.com>
(cherry picked from commit a187d83)
kdk pushed a commit that referenced this pull request Jan 7, 2020
* Fix backend config where lo ranges were not adjusted to Hz.

* Fix bug where assemble/execute accepted parameters in GHz rather than Hz.

* linting

* Documentation fixes for Hz.

* Add channel bandwidth.

* bug fix with kwarg pop.

* Missing None in kwargs pop.

* linting.

* Fix range in list comprehension for lo ranges in backend configuration.

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* lo to LO in docstrings.

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Lauren Capelluto <laurencapelluto@gmail.com>
(cherry picked from commit a187d83)

Co-authored-by: Thomas Alexander <thomasalexander2718@gmail.com>
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
* Fix backend config where lo ranges were not adjusted to Hz.

* Fix bug where assemble/execute accepted parameters in GHz rather than Hz.

* linting

* Documentation fixes for Hz.

* Add channel bandwidth.

* bug fix with kwarg pop.

* Missing None in kwargs pop.

* linting.

* Fix range in list comprehension for lo ranges in backend configuration.

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* lo to LO in docstrings.

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Lauren Capelluto <laurencapelluto@gmail.com>
@1ucian0 1ucian0 added the mod: pulse Related to the Pulse module label Jan 15, 2021
mergify bot pushed a commit that referenced this pull request Jan 15, 2021
* Fix backend config where lo ranges were not adjusted to Hz.

* Fix bug where assemble/execute accepted parameters in GHz rather than Hz.

* linting

* Documentation fixes for Hz.

* Add channel bandwidth.

* bug fix with kwarg pop.

* Missing None in kwargs pop.

* linting.

* Fix range in list comprehension for lo ranges in backend configuration.

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* Update qiskit/compiler/assemble.py

Co-Authored-By: Lauren Capelluto <laurencapelluto@gmail.com>

* lo to LO in docstrings.

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Lauren Capelluto <laurencapelluto@gmail.com>
(cherry picked from commit a187d83)

# Conflicts:
#	qiskit/assembler/assemble_schedules.py
#	qiskit/compiler/assemble.py
#	qiskit/execute.py
#	qiskit/providers/models/backendconfiguration.py
#	test/python/compiler/test_assembler.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Changelog: Bugfix Include in the "Fixed" section of the changelog mod: pulse Related to the Pulse module priority: high stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

qubit_lo_range and meas_lo_range are still in GHz in the configuration
6 participants