Cherry-Picked: Broken up QuantumSimulator into CommonNativeSimulator and QuantumSimulator #895
Conversation
kuzminrobin
requested review from
cgranade,
swernli,
bettinaheim and
DmitryVasilevsky
|
@kuzminrobin What is the intent of this change? Does the additional layer of indirection make the infrastructure more compatible with the sparse simulator? The changes seem reasonable, it's just hard to tell from the files what this enables. |
|
@swernli wrote:
Brief extract from an email that I have written to Bettina and Cassandra: |
|
@swernli wrote:
This is preparation for SparseSimulator, which has common part with QuantumSimulator. That common part is being extracted into a common parent - CommonNativeSimulator. |
| /// <summary> | ||
| /// The max number used as qubit id so far. | ||
| /// </summary> | ||
| public long MaxId { get; private set; } | ||
|
|
||
| public override Qubit CreateQubitObject(long id) | ||
| { | ||
| Debug.Assert(id < 50, "Using a qubit id > 50. This is a full-state simulator! Validating ids uniquenes might start becoming slow."); | ||
| Debug.Assert(id < 50, "Using a qubit id > 50. This is a full-state simulator! Validating ids uniqueness might start becoming slow."); |
There was a problem hiding this comment.
Sparse simulator may handle more than 50 qubits.
There was a problem hiding this comment.
So what should be the number here instead of 50?
There was a problem hiding this comment.
Theoretically there's no artificial limit. You may have millions of qubits as long as they are all in zero state. However, it's hard to imagine a meaningful program that would need that many all in the zero state. I believe there're tests that check functionality of the simulator with a few thousand qubits. But the point is - the check for full state simulator and sparse simulator should be substantially different. And the limit for sparse simulator should probably be around a few thousand.
| if (targets == null) throw new ArgumentNullException(nameof(targets), "Trying to perform an intrinsic operation on a null Qubit array."); | ||
| if (targets.Length == 0) throw new ArgumentNullException(nameof(targets), "Trying to perform an intrinsic operation on an empty Qubit array."); | ||
|
|
||
| bool[] used = new bool[((QSimQubitManager)QubitManager).MaxId]; |
There was a problem hiding this comment.
Here and in a few other places. For the full state simulator the maximum number of qubits were small. Sparse simulator can handle quite a few more in certain circumstances. It may not be always desirable to allocate array with the size of the number of qubits. I would consider array for a full state simulator and use HashSet for the sparse simulator.
There was a problem hiding this comment.
I didn't touch the SparseSimulator for a few months and I don't remember exactly its difference from the QuantumSimulator. And before I get back to the SparseSimulator I also need to resolve the Dump functionality issue. So paying significant attention to the SparseSim at this stage seems to me less efficient than if I move forward now. I will get back to SparseSim later any way, and that's when I plan to finalize the changes.
If there are no critical objections, I would prefer to move forward with this PR at the moment.
There was a problem hiding this comment.
I'm OK leaving this as is, especially given that sparse simulator won't be used on more than a few thousand qubits. Maybe we should note this somewhere - comments or work item.
Cherry-picked to
mainthe feature-branch changes of these 2 PRs: