Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Fixed the SparseSimulator tests. #945

Merged
merged 1 commit into from
Feb 18, 2022
Merged

Fixed the SparseSimulator tests. #945

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
2 changes: 0 additions & 2 deletions src/Simulation/Simulators/CommonNativeSimulator/QubitManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ public QSimQubitManager(bool throwOnReleasingQubitsNotInZeroState = true, long q

public override Qubit CreateQubitObject(long id)
{
Debug.Assert(id < 50, "Using a qubit id > 50. This is a full-state simulator! Validating ids uniqueness might start becoming slow.");
Copy link
Contributor

@DmitryVasilevsky DmitryVasilevsky Feb 18, 2022

Choose a reason for hiding this comment

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

Consider keeping it in the full-state simulator.

Copy link
Contributor Author

@kuzminrobin kuzminrobin Feb 18, 2022

Choose a reason for hiding this comment

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

Since the SparseSimulator and QuantumSimulator share as much as possible, including the QubitManager, it seems to me easier for the shared code to be the same for both simulators.

Of course I can add a parameter to the constructor of the CommonNativeSimulator, QSimQubitManager, QubitManager, telling
either which simulator has instantiated the QubitManager
or whether the qubit number limitation should be validated,
but that seems to me not worth the effort and not giving much value but obscuring the code.

I can be wrong, I realize that :-). Let me know. For now I would leave as is. Would you still like me to keep it for the QuantumSimulator case?

Copy link
Contributor

@DmitryVasilevsky DmitryVasilevsky Feb 18, 2022

Choose a reason for hiding this comment

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

Agreed, it doesn't make sense to put this check in the common code. Is there a way to put it in the full state simulator code that's not shared with the sparse simulator? If it is not possible - I approve.

Copy link
Contributor Author

@kuzminrobin kuzminrobin Feb 18, 2022

Choose a reason for hiding this comment

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

Makes sense.

I double-checked, unfortunately
this function is only called from the code common for QuantumSimulator and SparseSimulator,
and the argument is also calculated in the common code,
and the returned value cannot be easily analyzed by the QuantumSimulator only.

So it seems more efficient to move forward with this change. Can you please actually approve the PR?


if (id >= this.MaxId)
{
this.MaxId = id + 1;
Expand Down