Skip to content

Conversation

@rich7420
Copy link
Contributor

Purpose of PR

Improve Test Coverage for Quantum Gates and Circuit Operations.
This PR provides two gates test of multi qubit gates (CNOT gate , Toffoli gate). This file will includes CNOT gate , Toffoli gate , SWAP gate and CSWAP gate.

Related Issues or PRs

Related to #604

Changes Made

  • Bug fix
  • New feature
  • Refactoring
  • Documentation
  • Test
  • CI/CD pipeline
  • Other

Breaking Changes

  • Yes
  • No

Checklist

  • Added or updated unit tests for all changes
  • Added or updated documentation for all changes
  • Successfully built and ran all unit tests or manual tests locally
  • PR title follows "MAHOUT-XXX: Brief Description" format (if related to an issue)
  • Code follows ASF guidelines

@guan404ming
Copy link
Member

Please help fix the CI error, thanks!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive test coverage for multi-qubit quantum gates (CNOT and Toffoli gates) as part 1 of a test suite that will eventually include SWAP and CSWAP gates. The tests verify gate behavior across multiple quantum computing backends (Qiskit, Cirq, Amazon Braket).

  • Implements parametrized tests for CNOT and Toffoli gate state transitions, double applications (verifying gate self-inverse properties), and behavior on multi-qubit circuits
  • Adds backend-aware helper functions to handle different qubit ordering conventions (little-endian vs big-endian)
  • Includes edge case testing for uninitialized circuits, invalid qubit indices, and cross-backend consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 24 to 90
def get_state_probability(results, target_state, num_qubits=1, backend_name=None):
"""
Calculate the probability of measuring a target state.

Args:
results: Dictionary of measurement results from execute_circuit()
target_state: Target state as string (e.g., "0", "1", "101") or int
num_qubits: Number of qubits in the circuit
backend_name: Name of the backend (for handling qubit ordering)

Returns:
Probability of measuring the target state
"""
if isinstance(results, list):
results = results[0]

total_shots = sum(results.values())
if total_shots == 0:
return 0.0

# Convert target_state to both string and int formats for comparison
if isinstance(target_state, str):
target_str = target_state
# Convert binary string to integer
target_int = int(target_state, 2) if target_state else 0
else:
target_int = target_state
# Convert integer to binary string
target_str = format(target_state, f"0{num_qubits}b")

# Handle backend-specific qubit ordering
# Qiskit uses little-endian (rightmost bit is qubit 0)
# Amazon Braket and Cirq use big-endian (leftmost bit is qubit 0)
if backend_name == "qiskit" and isinstance(target_str, str) and len(target_str) > 1:
# Reverse the string for Qiskit (little-endian)
target_str_qiskit = target_str[::-1]
else:
target_str_qiskit = target_str

target_count = 0
for state, count in results.items():
if isinstance(state, str):
# For Qiskit, compare with reversed string
if backend_name == "qiskit" and len(state) > 1:
if state == target_str_qiskit:
target_count = count
break
else:
if state == target_str:
target_count = count
break
else:
# For Cirq, use integer comparison
# Cirq uses big-endian, so the integer representation matches
if backend_name == "cirq":
if state == target_int:
target_count = count
break
else:
# For other backends, also try integer comparison
if state == target_int:
target_count = count
break

return target_count / total_shots


Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

The get_state_probability function is duplicated from test_single_qubit_gates.py with added backend-specific handling. Consider extracting this function to a shared utility module (e.g., testing/utils/qumat_helpers.py) to avoid code duplication and make it easier to maintain. This would follow the DRY (Don't Repeat Yourself) principle and ensure consistent behavior across test files.

Suggested change
def get_state_probability(results, target_state, num_qubits=1, backend_name=None):
"""
Calculate the probability of measuring a target state.
Args:
results: Dictionary of measurement results from execute_circuit()
target_state: Target state as string (e.g., "0", "1", "101") or int
num_qubits: Number of qubits in the circuit
backend_name: Name of the backend (for handling qubit ordering)
Returns:
Probability of measuring the target state
"""
if isinstance(results, list):
results = results[0]
total_shots = sum(results.values())
if total_shots == 0:
return 0.0
# Convert target_state to both string and int formats for comparison
if isinstance(target_state, str):
target_str = target_state
# Convert binary string to integer
target_int = int(target_state, 2) if target_state else 0
else:
target_int = target_state
# Convert integer to binary string
target_str = format(target_state, f"0{num_qubits}b")
# Handle backend-specific qubit ordering
# Qiskit uses little-endian (rightmost bit is qubit 0)
# Amazon Braket and Cirq use big-endian (leftmost bit is qubit 0)
if backend_name == "qiskit" and isinstance(target_str, str) and len(target_str) > 1:
# Reverse the string for Qiskit (little-endian)
target_str_qiskit = target_str[::-1]
else:
target_str_qiskit = target_str
target_count = 0
for state, count in results.items():
if isinstance(state, str):
# For Qiskit, compare with reversed string
if backend_name == "qiskit" and len(state) > 1:
if state == target_str_qiskit:
target_count = count
break
else:
if state == target_str:
target_count = count
break
else:
# For Cirq, use integer comparison
# Cirq uses big-endian, so the integer representation matches
if backend_name == "cirq":
if state == target_int:
target_count = count
break
else:
# For other backends, also try integer comparison
if state == target_int:
target_count = count
break
return target_count / total_shots
from testing.utils.qumat_helpers import get_state_probability

Copilot uses AI. Check for mistakes.
qumat.apply_cnot_gate(*gate_args)
else:
qumat.apply_toffoli_gate(*gate_args)
except (IndexError, ValueError, RuntimeError, Exception):
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

Catching bare Exception is overly broad and can mask unexpected errors. This makes debugging more difficult since it will silently catch programming errors like AttributeError or TypeError. Consider catching only the specific exceptions that are expected (e.g., IndexError, ValueError, RuntimeError) or let unexpected exceptions propagate to surface actual bugs.

Suggested change
except (IndexError, ValueError, RuntimeError, Exception):
except (IndexError, ValueError, RuntimeError):

Copilot uses AI. Check for mistakes.
qumat.apply_toffoli_gate(*gate_args)
results = qumat.execute_circuit()
assert results is not None
except (ValueError, RuntimeError, Exception):
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

Catching bare Exception is overly broad and can mask unexpected errors. This makes debugging more difficult since it will silently catch programming errors like AttributeError or TypeError. Consider catching only the specific exceptions that are expected (e.g., ValueError, RuntimeError) or let unexpected exceptions propagate to surface actual bugs.

Suggested change
except (ValueError, RuntimeError, Exception):
except (ValueError, RuntimeError):

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

I think this one make sense, wdyt?

qumat.apply_cnot_gate(*gate_args)
else:
qumat.apply_toffoli_gate(*gate_args)
except (IndexError, ValueError, RuntimeError, Exception):
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
except (IndexError, ValueError, RuntimeError, Exception):
except (IndexError, ValueError, RuntimeError, Exception):
# Intentionally ignore exceptions: test passes if gate raises error for invalid qubit indices

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

Also this one

Comment on lines +489 to +497
try:
if gate_name == "cnot":
qumat.apply_cnot_gate(*gate_args)
else:
qumat.apply_toffoli_gate(*gate_args)
results = qumat.execute_circuit()
assert results is not None
except (ValueError, RuntimeError, Exception):
pass
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

'except' clause does nothing but pass and there is no explanatory comment.

Suggested change
try:
if gate_name == "cnot":
qumat.apply_cnot_gate(*gate_args)
else:
qumat.apply_toffoli_gate(*gate_args)
results = qumat.execute_circuit()
assert results is not None
except (ValueError, RuntimeError, Exception):
pass
# The gate should either raise an error (invalid qubit indices) or execute gracefully.
# We explicitly check for exceptions using pytest.raises, and if no exception is raised,
# we assert that the results are not None.
exception_types = (ValueError, RuntimeError, Exception)
try:
if gate_name == "cnot":
qumat.apply_cnot_gate(*gate_args)
else:
qumat.apply_toffoli_gate(*gate_args)
except exception_types:
# Exception was raised as expected for invalid/same qubit indices.
return
results = qumat.execute_circuit()
assert results is not None

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

This as well

@rawkintrevo
Copy link
Contributor

what is status here?

@rich7420
Copy link
Contributor Author

I don't know if we need DRY way like copliot said.

@guan404ming
Copy link
Member

Sorry for the late, I got lots of deadline this week🔥

Copy link
Member

@guan404ming guan404ming left a comment

Choose a reason for hiding this comment

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

Overall looks good, left some comments

@guan404ming
Copy link
Member

I'm going to merge this one and I think we could refine this if wanted.

@guan404ming guan404ming merged commit f210fa4 into apache:main Nov 26, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants