Skip to content

Fix modification of rotation matrix in Givens rotation#1379

Open
arettig wants to merge 1 commit into
quantumlib:mainfrom
arettig:givens_rot_fix
Open

Fix modification of rotation matrix in Givens rotation#1379
arettig wants to merge 1 commit into
quantumlib:mainfrom
arettig:givens_rot_fix

Conversation

@arettig

@arettig arettig commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #761 (supersedes #1344). In the optimal_givens_decomposition() function, the rotation matrix was modified, leading to different results when the function was called again with the same rotation matrix. The function now makes a copy of the rotation matrix parameter to avoid this issue. A test was added to ensure consecutive calls to optimal_givens_decomposition() yield identical results.

In the Givens rotation function, the rotation matrix was modified,
leading to different results when the function was called again with
the same rotation matrix. The function now makes a copy of the matrix.
A test was added to ensure subsequent calls are identical.
@arettig arettig requested a review from mhucka June 25, 2026 00:01
@arettig arettig added the priority/before-1.7.2 Things to do before the next release label Jun 25, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request modifies optimal_givens_decomposition to copy the input unitary matrix before processing, preventing side effects from in-place modifications. A corresponding unit test, test_consecutive_optimal_givens_decomposition, has been added to verify that consecutive calls with the same matrix produce identical circuits. There are no review comments, and I have no additional feedback to provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/before-1.7.2 Things to do before the next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

optimal_givens_decomposition has bad side effects

2 participants