Skip to content

Conversation

alexanderivrii
Copy link
Member

@alexanderivrii alexanderivrii commented Mar 24, 2025

Summary

Fixes #14074.

The basis_translator pass was not always updating the DAG's global phase as a part of replace_node: this was only done when the target dag's global phase matched Param::ParameterExpression. Updating the global phase in the case of Param::Float was missing, and this is fixed now.

I am somewhat unsure if we also need to handle the Param::Obj case. I don't understand in what kind of a circuit we would see this, and I don't believe it's even supported, as per dag_circuit.rs.add_global_phase, which throws an error in the case of Param::Obj. Update: @jakelishman confirmed that Param::Obj is never permitted in global_phase.

Update: added release notes.

@alexanderivrii alexanderivrii requested a review from a team as a code owner March 24, 2025 12:52
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@alexanderivrii alexanderivrii added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Mar 24, 2025
@alexanderivrii alexanderivrii added this to the 2.1.0 milestone Mar 24, 2025
@coveralls
Copy link

coveralls commented Mar 24, 2025

Pull Request Test Coverage Report for Build 14036974943

Details

  • 25 of 29 (86.21%) changed or added relevant lines in 1 file are covered.
  • 25 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.03%) to 88.049%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/accelerate/src/basis/basis_translator/mod.rs 25 29 86.21%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/unitary_synthesis.rs 1 94.79%
crates/qasm2/src/lex.rs 6 92.73%
crates/qasm2/src/parse.rs 18 96.68%
Totals Coverage Status
Change from base Build 14034596400: -0.03%
Covered Lines: 72609
Relevant Lines: 82464

💛 - Coveralls

Copy link
Contributor

@raynelfss raynelfss left a comment

Choose a reason for hiding this comment

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

LGTM! Happy merging 🚀

@raynelfss raynelfss modified the milestones: 2.1.0, 2.0.0 Mar 25, 2025
@raynelfss raynelfss added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Mar 25, 2025
@raynelfss raynelfss added this pull request to the merge queue Mar 25, 2025
@raynelfss
Copy link
Contributor

@Mergifyio backport stable/1.4 stable/2.0

Copy link
Contributor

mergify bot commented Mar 25, 2025

backport stable/1.4 stable/2.0

✅ Backports have been created

Merged via the queue into Qiskit:main with commit d67c818 Mar 25, 2025
21 checks passed
mergify bot pushed a commit that referenced this pull request Mar 25, 2025
* add missing branch for global phase update in basis_translator

* Adding test

* reno

(cherry picked from commit d67c818)

# Conflicts:
#	crates/accelerate/src/basis/basis_translator/mod.rs
#	test/python/transpiler/test_basis_translator.py
mergify bot pushed a commit that referenced this pull request Mar 25, 2025
* add missing branch for global phase update in basis_translator

* Adding test

* reno

(cherry picked from commit d67c818)

# Conflicts:
#	crates/accelerate/src/basis/basis_translator/mod.rs
github-merge-queue bot pushed a commit that referenced this pull request Mar 25, 2025
…14092)

* Fix global phase update in `BasisTranslator` Pass (#14078)

* add missing branch for global phase update in basis_translator

* Adding test

* reno

(cherry picked from commit d67c818)

# Conflicts:
#	crates/accelerate/src/basis/basis_translator/mod.rs

* Fix: Add missing py token in `add_global_phase`.

---------

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Raynel Sanchez <raynelfss@hotmail.com>
github-merge-queue bot pushed a commit that referenced this pull request Mar 26, 2025
…14091)

* Fix global phase update in `BasisTranslator` Pass (#14078)

* add missing branch for global phase update in basis_translator

* Adding test

* reno

(cherry picked from commit d67c818)

# Conflicts:
#	crates/accelerate/src/basis/basis_translator/mod.rs
#	test/python/transpiler/test_basis_translator.py

* Fix: Add py token to `add_global_phase`

* Update test_basis_translator.py

* Fix: Use `new_bound` methods.

---------

Co-authored-by: Alexander Ivrii <alexi@il.ibm.com>
Co-authored-by: Raynel Sanchez <raynelfss@hotmail.com>
Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Co-authored-by: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com>
@raynelfss raynelfss mentioned this pull request Mar 28, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog 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.

Transplie Rx introduce an incorrect global phase.
4 participants