-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Avoid lossing precision when scaling frequencies (backport #12392) #12416
Conversation
* Avoid lossing precision when scaling frequencies Classes in pulse_instruction.py scale frequency values to GHz by multipliying `ParameterExpression` with float 1e9. This can lead to numerical errors on some systems using symengine. Instead, this scaling can be done multiplying by integer 10**9. See: #12359 (comment) * Add release note --------- Co-authored-by: Jake Lishman <jake.lishman@ibm.com> (cherry picked from commit 96607f6)
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 9109206127Details
💛 - Coveralls |
@Mergifyio backport stable/0.46 |
✅ Backports have been created
|
* Avoid lossing precision when scaling frequencies Classes in pulse_instruction.py scale frequency values to GHz by multipliying `ParameterExpression` with float 1e9. This can lead to numerical errors on some systems using symengine. Instead, this scaling can be done multiplying by integer 10**9. See: #12359 (comment) * Add release note --------- Co-authored-by: Jake Lishman <jake.lishman@ibm.com> (cherry picked from commit 96607f6) Co-authored-by: Iyán <me@iyanmv.com> (cherry picked from commit 0a05ea2)
Summary
Classes in
pulse_instruction.py
scale frequency values to GHz by multipliyingParameterExpression
with float1e9
. This can lead to numerical errors on some systems due tosymengine
"rounding" errors. Instead, this scaling can be done multiplying by integer10**9
.Details and comments
In this unit test
qiskit/test/python/qobj/test_pulse_converter.py
Line 343 in 235e581
The
frequency
string"f / 1000"
gets converted toParameterExpression(1000000.0*f)
afterParameterExpression(f/1000)
is multiplied by1e9
. For some unknown reason, when the symbolf
is later substituted with the value3.14
, and theRealDouble
is converted tofloat
, an error is introduced that can't be fixed byqiskit/qiskit/pulse/utils.py
Lines 71 to 74 in 235e581
This fixes: #12359 (comment)
Upstream issue: symengine/symengine.py#476
This is an automatic backport of pull request #12392 done by Mergify.