Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CQT-360] QX simulator outputs invalid states #205

Merged
merged 4 commits into from
Mar 31, 2025

Conversation

rturrado
Copy link
Contributor

@rturrado rturrado commented Mar 28, 2025

The problem came from SparseComplex::operator=, which was not always performing the assignment (only when a check was true, and that check was wrong).

Implement operator<< for qx::Measurement and qx::SuperposedState.
Add fmt::formatters for qx::Measurement and qx::SuperposedState.

Remove rule of 5 for SparseComplex.
Remove SparseArray::operator=(MapBasisVectorToSparseComplex).

…seems not to erase previous elements of 'state', thus we force 'state.data_.clear()' in SparseArray::operator=(const SparseArray& other).

Add fmt::formatters for qx::Measurement and qx::SuperposedState.

Implement rule of 5 for SparseArray and QuantumState.
Implement SparseArray::operator=(const SparseArray& other).
Implement operator<< for qx::Measurement and qx::SuperposedState.

Fix SparseComplex::SparseComplex(SparseComplex&& other).

Remove SparseArray::operator=(MapBasisVectorToSparseComplex map).
@rturrado rturrado requested a review from elenbaasc March 28, 2025 21:21
… which was not always performing the assignment (only when a check was true, and that check was wrong).

Remove rule of 5 for QuantumState, SparseArray and SparseComplex.
Copy link
Contributor

@elenbaasc elenbaasc left a comment

Choose a reason for hiding this comment

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

Thanks for checking this out, figuring out the bug, and making a fix!!

Was the rule of 5 for SparseComplex added by you recently?

Should we still add the test circuit to this PR (i.e. the one that showed the bug, that started with H q[1])?

@elenbaasc
Copy link
Contributor

Testcase:

Circuit

version 3.0

qubit[2] q

H q[1]
ctrl.X90 q[0], q[1]
H q[0]
H q[1]

Results

{
  "00": { "real": 0.7071067812, "imag": 0.0, "norm": 0.5 },
  "01": { "real": 0.7071067812, "imag": 0.0, "norm": 0.5 },
  "10": { "real": 0.0,          "imag": 0.0, "norm": 0.0 },
  "11": { "real": 0.0,          "imag": 0.0, "norm": 0.0 }
}

The results would be the same for the circuit where ctrl.X90 is replaced by ctrl.Rx(pi/2).

@rturrado
Copy link
Contributor Author

Thanks for checking this out, figuring out the bug, and making a fix!!

Thanks!

Was the rule of 5 for SparseComplex added by you recently?

I think I added it a few weeks ago with the following idea: whenever the source of the assignment is below epsilon, i.e., not really null but could be considered null, we just set null in the destination. But the code to do the check for null was wrong. I could have fixed that code, but I decided to remove everything altogether. So now an assignment really makes a copy.

Should we still add the test circuit to this PR (i.e. the one that showed the bug, that started with H q[1])?

Yes, please. I have already prepared it. But I need the expected results.

@rturrado
Copy link
Contributor Author

Testcase:

Circuit

version 3.0

qubit[2] q

H q[1]
ctrl.X90 q[0], q[1]
H q[0]
H q[1]

Results

{
  "00": { "real": 0.7071067812, "imag": 0.0, "norm": 0.5 },
  "01": { "real": 0.7071067812, "imag": 0.0, "norm": 0.5 },
  "10": { "real": 0.0,          "imag": 0.0, "norm": 0.0 },
  "11": { "real": 0.0,          "imag": 0.0, "norm": 0.0 }
}

The results would be the same for the circuit where ctrl.X90 is replaced by ctrl.Rx(pi/2).

Oh beautiful, thanks so much.

Copy link
Contributor

@elenbaasc elenbaasc left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@rturrado rturrado merged commit 1ef8fb4 into develop Mar 31, 2025
20 checks passed
@rturrado rturrado deleted the CQT-360-QX-simulator-outputs-invalid-states branch March 31, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants