Closed
Conversation
orpuente-MS
requested review from
billti,
idavis and
minestarks
as code owners
billti
reviewed
Feb 24, 2026
minestarks
reviewed
Apr 13, 2026
| } | ||
| } | ||
|
|
||
| #[ignore = "loss behavior in cpu and gpu simulators is different"] |
Member
There was a problem hiding this comment.
What's the plan with the ignore? Is this meant to stay here indefinitely or is this test going to be updated?
Contributor
Author
There was a problem hiding this comment.
I have a branch fixing this difference in loss behavior. But I was waiting for this PR to merge so that I could test the changes on that branch.
minestarks
reviewed
Apr 13, 2026
| /// ``` | ||
| macro_rules! check_programs_are_eq { | ||
| // Pattern without num_results - defaults to 0 | ||
| // Multi-simulator entry: runs the same test on each listed simulator |
Member
There was a problem hiding this comment.
I'm sorry, I'm just not seeing the benefit of using macros so extensively. I find the syntax so difficult to follow and debug. If I'm the only one that feels this way about macros, I'm happy to be wrong - but I'd be surprised any of us would be able to read this easily without consulting documentation.
Copilot suggested something like the following to support dispatching based on simulator type. It's shorter and more readable... why go for something more complex?
pub enum Sim {
Noiseless,
Noisy,
Stabilizer,
Gpu,
}
pub fn check_programs_are_eq(
simulators: &[Sim],
programs: &[Vec<QirInstruction>],
num_qubits: u32,
num_results: u32,
) {
use qdk_simulators::cpu_full_state_simulator::{NoiselessSimulator, NoisySimulator};
use qdk_simulators::stabilizer_simulator::StabilizerSimulator;
for sim in simulators {
match sim {
Sim::Noiseless => {
check_programs_are_eq_cpu::<NoiselessSimulator>(programs, num_qubits, num_results);
}
Sim::Noisy => {
check_programs_are_eq_cpu::<NoisySimulator>(programs, num_qubits, num_results);
}
Sim::Stabilizer => {
check_programs_are_eq_cpu::<StabilizerSimulator>(programs, num_qubits, num_results);
}
Sim::Gpu => {
if gpu_is_available() {
check_programs_are_eq_gpu(programs, num_qubits, num_results);
} else {
eprintln!("Skipping GPU test: no compatible GPU adapter found");
}
}
}
}
}
minestarks
reviewed
Apr 13, 2026
| @@ -732,6 +993,49 @@ where | |||
| /// } | |||
| /// ``` | |||
| macro_rules! check_basis_table { | |||
Member
There was a problem hiding this comment.
Same comment about the macros... wouldn't this be more readable as a normal function? (Taking advantage of the same simulator type enum from the other comment)
pub fn check_basis_table(
simulators: &[Sim],
num_qubits: u32,
table: &[(Vec<QirInstruction>, u32, u32)],
) {
...
}
Member
|
@orpuente-MS I get a ton of error messages running these tests locally, but no failing tests. What gives? This is on my Windows devbox VM |
Contributor
Author
|
Closing this PR in favor of #3124. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR extends the work in #2905 by adding tests for the GPU simulator. It also unifies the tests for all simulators in this test-suite, which serves as a mechanism to verify that they all behave the same.