Refactor Simulator Heirarchy + Make Hardware Requirements Clearer in Experiments.py#783
Refactor Simulator Heirarchy + Make Hardware Requirements Clearer in Experiments.py#783jasonhan3 wants to merge 10 commits into
Conversation
…o make experiments.py contract for hardware type clearer
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
☂️ Code Coverage
Overall Coverage
New Files
Modified Files
|
…tor-task chore: merged 'main' into branch
weinbe58
left a comment
There was a problem hiding this comment.
Just a few comments. In particular the property to get the physical squin kernel needs to be checked to make sure its actually OK to use the logical Move to Squin in stead of the physical version.
| from bloqade.lanes.transform import MoveToSquinLogical | ||
|
|
||
| return MoveToSquinLogical( | ||
| arch_spec=self.physical_arch_spec, | ||
| noise_model=self.noise_model, | ||
| add_noise=True, | ||
| ).emit(self.physical_move_kernel) |
There was a problem hiding this comment.
Note that this step might also need to be abstracted because the logical simulator and the physical simulator rewrites are slightly different.
There was a problem hiding this comment.
I added two tests on a physical squin kernel for the noisy/noiseless rewrites here: 9a5df03
I didn't add more tests to test every single statement for the InsertGates and InsertNoise rewrite passes (I can do this if desired), but functionally, I think it should be the same because I think the only difference between MoveToSquinLogical and MoveToSquinPhysical is that "initialize_kernel" is passed into the InsertGates rewriter and "initialize_noise_kernel" is passed into the InsertNoise rewriter. However, those kernels are only used if the rewriter encounters a move.PhysicalInitialize statement, which should not exist if a user is programming in physical squin. So the functionality should remain the same.
However, there is a "con" to this, which is that if a user wants to supply their custom noise model to a PhysicalSimulator, then they have to supply a LogicalNoiseModelABC which defines a "get_logical_initialize" method. Because this method should never be used for the PhysicalSimulator, they have to basically define a kernel that takes in (theta, phi, lam, qubits) and just returns None (a no-op kernel) to satisfy pyright.
However, to get around this "con", I am wondering if we can improve the abstractions for the noise model (for example, instead of having a LogicalNoiseModelABC class, can we just rewrite move.PhysicalInitialize into gate statements, and now that those are all statements in the move IR, like move.LocalRz, move.GlobalR, etc.), we can then call the InsertNoise pass on that? So we can get rid of LogicalNoiseModelABC altogether, and just use one MoveToSquin class. (As opposed to keeping the current LogicalNoiseModelABC design and defining additional abstractions on the Logical/Physical simulator tasks). I wonder if we can reduce some abstraction added by doing something like this.
edit: If this is too much to do/consider in this PR, I can also just introduce the additional abstraction for now, and then add a deprecation warning if we ever want to simplify the internal abstractions.
| def _apply_special_tsim_circuit_strategy( | ||
| task_map: Mapping[str, _TaskT], | ||
| ) -> dict[str, _TaskT]: | ||
| """Prepend the inverse compiled unitary prefix to each task's circuits.""" | ||
|
|
||
| transformed = dict(task_map) | ||
| for task in transformed.values(): | ||
| compiled_prefix = task.tsim_circuit[ | ||
| : _first_nonunitary_instruction_index(task.tsim_circuit) | ||
| ] | ||
| inverse_prefix = compiled_prefix.without_noise().inverse() | ||
| special_circuits = _apply_noiseless_unitary_prefix( | ||
| {basis: task.stim_circuit for basis, task in transformed.items()} | ||
| ) | ||
| special_noiseless_circuits = _apply_noiseless_unitary_prefix( | ||
| {basis: task.noiseless_stim_circuit for basis, task in transformed.items()} | ||
| ) | ||
| for basis, task in transformed.items(): | ||
| _clear_task_tsim_artifacts(task) | ||
| task.__dict__["tsim_circuit"] = inverse_prefix + task.tsim_circuit | ||
| task.__dict__["noiseless_tsim_circuit"] = ( | ||
| inverse_prefix + task.noiseless_tsim_circuit | ||
| ) | ||
| vars(task)["stim_circuit"] = special_circuits[basis] | ||
| vars(task)["noiseless_stim_circuit"] = special_noiseless_circuits[basis] |
There was a problem hiding this comment.
No we don't need this function (_apply_special_tsim_circuit_strategy) at all in the current experiments code (as I redesigned it slightly); I kept it for "backwards compatibility" I suppose. But it's a private function so I can delete it
…for noisy/noiseless cases
…tor-task chore: merged 'main' into branch
|
Maybe we can hold off on merging this for now; talking about the changes with Stefan. |
Addresses the first bullet point in #765.
TLDR
Refactored simulator and simulator task heirarchy to account for the physical/logical compilation paths as well as different simulation backends. With the refactor, also attempted to make hardware requirements clearer in experiments.py for the MSD experiments notebook.
Overall Design Decision
Ideally, we want to reuse as much as the existing "GeminiLogicalSimulator" and "GeminiLogicalSimulatorTask" code as possible while minimizing extra abstraction/complexity added. "GeminiLogicalSimulator" is pretty generic except for the fact that physical/logical compilation pipeline is different (compile_task function in GeminiLogicalSimulator vs. using PhysicalPipeline for physical squin kernels).
Use cases: Physical/Logical Compilation, and Different Simulation Backends
The two big questions are:
For this PR, the answers, respectively, are:
- Implicit here is the question of, "How do we want the user to interface with the simulator"? We can either have the user construct different simulator classes based on the simulation backend to use, or we could just have one class that is configurable with a string that specifies the simulation backend (something like, Simulator(backend="tsim")). We choose the former in this PR, thus leading us to define more concrete classes.
NOTE: I am basically delegating the concerns in the simulator/simulatortask heirarchy as follows:
Simulator/SimulatorTask Class Heirarchy
As a result of the answers to the above questions, the design in this PR is the following:
Simulator Heirarchy
AbstractSimulator (top-level; defines the shared simulator functionalities)
PhysicalSimulator/PhysicalCliffTSimulator/GeminiLogicalSimulator/GeminiLogicalCliffTSimulator
- Alternative design decision: alternatively, for (2), one could observe that all that we actually need is the simulator task type; the rest of the code is identical if we change simulation backends. Due to type erasure, we unfortunately can't use type parameters to construct a simulator task object of our desired type. An alternative design would be to pass in the simulation task class when constructing the simulator (which the simulator class could use), but this might be awkward in terms of usability, which is why I did not choose to implement that in this PR.
SimulatorTask Heirarchy
AbstractSimulatorTask (top-level; defines the shared simulationtask functionalities)
CliffTSimulatorTask/TsimSimulatorTask
GeminiLogicalSimulatorTask/GeminiLogicalCliffTSimulatorTask/PhysicalSimulatorTask/PhysicalCliffTSimulatorTask
- Avoids breaking changes (PhysicalSimulatorTask and GeminiLogicalSimulatorTask both currently exist)
- Adding GeminiLogicalCliffTSimulatorTask and PhysicalCliffTSimulatorTask, although we can delete them later, is more consistent for now given that we do support PhysicalSimulatorTask and GeminiLogicalSimulatorTask (defining simulatortask's for each combination of logical/physical compilation + simulation backend).
Implementation Details
- Thus, for the user's perspective, when they have to construct a LogicalNoiseModelABC, they can just define empty squin kernels for the noiseless/noisy squin kernels for the initialize kernels (assuming they will never have the move.PhysicalInitialize statement in their kernels)
Breaking changes
Future Breaking Changes
Features To Add
Changes to Experiments.py