Skip to content

Fix global phase update in BasisTranslator Pass - #14078

Merged
Raynel Sanchez (raynelfss) merged 3 commits into
Qiskit:mainfrom
alexanderivrii:fix-global-phase
Mar 25, 2025
Merged

Raynel Sanchez (raynelfss) merged 3 commits into
Qiskit:mainfrom
alexanderivrii:fix-global-phase

Conversation

@alexanderivrii

@alexanderivrii Alexander Ivrii (alexanderivrii) commented Mar 24, 2025 •

Copy link
Copy Markdown
Member

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: Jake Lishman (@jakelishman) confirmed that Param::Obj is never permitted in global_phase.

Update: added release notes.

@qiskit-bot

Copy link
Copy Markdown
Collaborator

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

  • @Qiskit/terra-core

@coveralls

Coveralls (coveralls) commented Mar 24, 2025 •

Copy link
Copy Markdown

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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Happy merging 🚀

@raynelfss Raynel Sanchez (raynelfss) added the stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge. label Mar 25, 2025
@raynelfss
Raynel Sanchez (raynelfss) added this pull request to the merge queue Mar 25, 2025
@raynelfss

Copy link
Copy Markdown
Contributor

Mergify (@Mergifyio) backport stable/1.4 stable/2.0

@mergify

mergify Bot commented Mar 25, 2025 •

Copy link
Copy Markdown
Contributor

backport stable/1.4 stable/2.0

✅ Backports have been created

Details

Merged via the queue into Qiskit:main with commit d67c818 Mar 25, 2025
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 Raynel Sanchez (raynelfss) mentioned this pull request Mar 28, 2025
4 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transplie Rx introduce an incorrect global phase.

4 participants