Skip to content

Use local random seeds in slater determinant tests #1308

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.

Sign up for GitHub

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

Open
mzl2233 wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −8
Open

Use local random seeds in slater determinant tests #1308

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions src/openfermion/circuits/slater_determinants_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand Down Expand Up @@ -137,13 +137,16 @@
class JWGetGaussianStateTest(unittest.TestCase):
def setUp(self):
self.n_qubits_range = range(2, 10)
self.rng = numpy.random.default_rng(5387)
self.hamiltonian_seed_sequence = numpy.random.SeedSequence(5387)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 14, 2026

Choose a reason for hiding this comment

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

high

The SeedSequence is redundant here because self.rng (a Generator) can be used to produce seeds for the Hamiltonian construction. Additionally, using the same seed (5387) for both default_rng and SeedSequence can lead to correlated random streams. It is recommended to use a single generator for all random choices in the test to ensure independence and simplify the code.

Suggested change
self.hamiltonian_seed_sequence = numpy.random.SeedSequence(5387)
# self.hamiltonian_seed_sequence is not needed if we use self.rng


def test_ground_state_particle_conserving(self):
"""Test getting the ground state of a Hamiltonian that conserves
particle number."""
for n_qubits in self.n_qubits_range:
# Initialize a particle-number-conserving Hamiltonian
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True)
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True, seed=hamiltonian_seed)
Comment on lines +148 to +149
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 14, 2026

Choose a reason for hiding this comment

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

high

Calling spawn(1) inside the loop returns the same child SeedSequence in every iteration, which results in the same hamiltonian_seed being used for all values of n_qubits. This reduces the diversity of the test cases. Use the existing self.rng to generate a unique seed for each iteration.

Suggested change
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True, seed=hamiltonian_seed)
hamiltonian_seed = int(self.rng.integers(0, 2**32))
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True, seed=hamiltonian_seed)


# Compute the true ground state
sparse_operator = get_sparse_operator(quadratic_hamiltonian)
Expand All @@ -165,7 +168,8 @@
conserve particle number."""
for n_qubits in self.n_qubits_range:
# Initialize a non-particle-number-conserving Hamiltonian
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False)
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False, seed=hamiltonian_seed)
Comment on lines +171 to +172
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 14, 2026

Choose a reason for hiding this comment

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

high

Calling spawn(1) inside the loop returns the same child SeedSequence in every iteration, which results in the same hamiltonian_seed being used for all values of n_qubits. Use the existing self.rng to generate a unique seed for each iteration.

Suggested change
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False, seed=hamiltonian_seed)
hamiltonian_seed = int(self.rng.integers(0, 2**32))
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False, seed=hamiltonian_seed)


# Compute the true ground state
sparse_operator = get_sparse_operator(quadratic_hamiltonian)
Expand All @@ -187,11 +191,12 @@
particle number."""
for n_qubits in self.n_qubits_range:
# Initialize a particle-number-conserving Hamiltonian
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True)
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True, seed=hamiltonian_seed)
Comment on lines +194 to +195
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 14, 2026

Choose a reason for hiding this comment

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

high

Calling spawn(1) inside the loop returns the same child SeedSequence in every iteration, which results in the same hamiltonian_seed being used for all values of n_qubits. Use the existing self.rng to generate a unique seed for each iteration.

Suggested change
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True, seed=hamiltonian_seed)
hamiltonian_seed = int(self.rng.integers(0, 2**32))
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, True, seed=hamiltonian_seed)


# Pick some orbitals to occupy
num_occupied_orbitals = numpy.random.randint(1, n_qubits + 1)
occupied_orbitals = numpy.random.choice(range(n_qubits), num_occupied_orbitals, False)
num_occupied_orbitals = self.rng.integers(1, n_qubits + 1)
occupied_orbitals = self.rng.choice(range(n_qubits), num_occupied_orbitals, False)

# Compute the Gaussian state
circuit_energy, gaussian_state = jw_get_gaussian_state(
Expand Down Expand Up @@ -219,11 +224,12 @@
particle number."""
for n_qubits in self.n_qubits_range:
# Initialize a non-particle-number-conserving Hamiltonian
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False)
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False, seed=hamiltonian_seed)
Comment on lines +227 to +228
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 14, 2026

Choose a reason for hiding this comment

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

high

Calling spawn(1) inside the loop returns the same child SeedSequence in every iteration, which results in the same hamiltonian_seed being used for all values of n_qubits. Use the existing self.rng to generate a unique seed for each iteration.

Suggested change
hamiltonian_seed = self.hamiltonian_seed_sequence.spawn(1)[0].generate_state(1)[0]
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False, seed=hamiltonian_seed)
hamiltonian_seed = int(self.rng.integers(0, 2**32))
quadratic_hamiltonian = random_quadratic_hamiltonian(n_qubits, False, seed=hamiltonian_seed)


# Pick some orbitals to occupy
num_occupied_orbitals = numpy.random.randint(1, n_qubits + 1)
occupied_orbitals = numpy.random.choice(range(n_qubits), num_occupied_orbitals, False)
num_occupied_orbitals = self.rng.integers(1, n_qubits + 1)
occupied_orbitals = self.rng.choice(range(n_qubits), num_occupied_orbitals, False)

# Compute the Gaussian state
circuit_energy, gaussian_state = jw_get_gaussian_state(
Expand Down
Loading