Feature/io simulation#22
Open
Bloooman wants to merge 38 commits into
Open
Conversation
…ile. Add ADT name to all vairants
…erformed on a QPU
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #22 +/- ##
===========================================
- Coverage 78.26% 48.99% -29.27%
===========================================
Files 4 10 +6
Lines 184 696 +512
===========================================
+ Hits 144 341 +197
- Misses 40 355 +315 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Krastanov
requested changes
May 27, 2026
Member
Krastanov
left a comment
Krastanov
left a comment
There was a problem hiding this comment.
here are a few comments on datastructures and typetree -- they are my main concern
the other thing I want to discuss before merging is the split into kernels, traversals, filters that we have discussed, mainly in terms of documentation and enforecement
Comment on lines
+62
to
+82
| """ADT representing different types of measurement result""" | ||
| @data MeasurementResultType begin | ||
| """Denoting measurement results that classically determined by a coin flip""" | ||
| ClassicalDetermRes | ||
| """Denoting measurement results that are classically determined by stored eigenvalues of stabilizers""" | ||
| ClassicalRandomRes | ||
| """Denoting measurement results that require performing actual quantum measurement""" | ||
| QuantumRes | ||
| end | ||
|
|
||
| using .MeasurementResultType: ClassicalDetermRes, ClassicalRandomRes, QuantumRes | ||
|
|
||
| """Struct holding measurement result value and its type""" | ||
| struct MeasurementResult | ||
| """Corresponding Pauli String of Measurement""" | ||
| pauli::PauliOperator | ||
| """Single bit measurement result in boolean""" | ||
| result::Union{Bool,Nothing} | ||
| """Measurement result type of this result (ClassicalDetermRes, ClassicalRandomRes, QuantumRes)""" | ||
| result_type::MeasurementResultType.Type | ||
| end |
Member
There was a problem hiding this comment.
this should probably should be a single ADT
Comment on lines
+86
to
+91
| """TODO docstring""" | ||
| classical_deterministic_result(p::PauliOperator, m::Union{Bool,Nothing}) = MeasurementResult(p, m, ClassicalDetermRes()) | ||
| """TODO docstring""" | ||
| classical_random_result(p::PauliOperator, m::Union{Bool,Nothing}) = MeasurementResult(p, m, ClassicalRandomRes()) | ||
| """TODO docstring""" | ||
| quantum_result(p::PauliOperator, m::Union{Bool,Nothing}) = MeasurementResult(p, m, QuantumRes()) |
Member
There was a problem hiding this comment.
These should just be constructors
Comment on lines
+135
to
+139
| """TODO docstring""" | ||
| abstract type QuantumRuntime end | ||
|
|
||
| """TODO docstring -- all measurements return `nothing` and classically-trackable states are set as if result was `false`.""" | ||
| struct MockRuntime <: QuantumRuntime end |
Member
There was a problem hiding this comment.
delete if not using, it seems your AbstractSimState and MemoryState have taken over this
Comment on lines
+94
to
+122
| """Struct that contains information describing current quantum state""" | ||
| struct MemoryState | ||
| """Vector that holds all MeasurementResult""" | ||
| measurement_results::Vector{MeasurementResult} | ||
| """Stabilizer object that describes current quantum state""" | ||
| stabilizer_group::Stabilizer | ||
| """GeneralizedStabilizer object holding current quantum state within quantum computer""" | ||
| quantum_memory::Union{GeneralizedStabilizer, Nothing} | ||
| """Vector that holds all classical bits storing corresponding measurement results""" | ||
| classical_register::Vector{Union{Nothing,Bool}} | ||
| end | ||
|
|
||
| """Abstract base type for simulator execution states.""" | ||
| abstract type AbstractSimState end | ||
|
|
||
| """Execution state for real backend simulation.""" | ||
| struct SimState <: AbstractSimState | ||
| """Contain current circuit object""" | ||
| circuit::Circuit | ||
| """Number of gadgets inserted to replace nonclifford circuit operations""" | ||
| num_gadgets::Int | ||
| """Denote the Pauli Product Measurement that is being processed""" | ||
| instruction_pointer::Int | ||
| """Contain current quantum state""" | ||
| memory_state::MemoryState | ||
| end | ||
|
|
||
| """Execution state for dummy simulation. No real backend is contacted.""" | ||
| struct DummyState <: AbstractSimState |
Member
There was a problem hiding this comment.
struct MemoryState
# I assume this is something to be used by the compiler, not by the simulator
# i.e. this guy should not contain anything related to a specific runtime backend (simulator/dummy/actual quantum computer)
# e.g. quantum_memory should be inside SimState
suggested renamings
MemoryState -> CompilerState
AbstractSimState -> AbstractRuntime
SimState -> SimRuntime
DummyState -> DummyRuntime
And the common features of AbstractRuntime should probably go to CompilerState
No need to follow this exactly, but the goal should be to split "compiler" from "how is the runtime happening, e.g. dummy vs sim vs actual hardware"
Member
There was a problem hiding this comment.
this should probably not be in git?
Member
|
for kernels specify whether they are "simple" in some sense or do very significant manipulations (e.g. turning one gate to one gate, or to many gates, or two to two, or delete) so that we know how to design a general-purpose traversal |
Bloooman
force-pushed
the
feature/io_simulation
branch
from
6de4ace to
ab36b05
Compare
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
2 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.
Parser functions for OpenQASM 2.0 and functions save compiler outputs
Compute and compile input circuit using QuantumClifford backend or dummy backend
Perform joint measurement on Pauli Product Measurements and resolve bit conditional circuit operations
Before considering your pull request ready for review and merging, make sure that all of the following are completed (please keep the clecklist as part of your PR):
If you are submitting for a bug bounty:
If possible, keep your git history not too wild (rebase and squash commits, keep commits small and semantically separated) so that review is easier.