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

Accepting circuits as initial states #817

Merged
merged 5 commits into from
Mar 7, 2023

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Mar 6, 2023

This PR implements the following point:

  • modify circuit execution to accept initial states of two types: arrays and circuits (to be able to execute on hardware).

from #720 and PR #799.
I will open a PR in qibolab to support this feature.

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

Verified

This commit was created on github.com and signed with GitHub’s verified signature.
@andrea-pasquale andrea-pasquale self-assigned this Mar 6, 2023
@andrea-pasquale andrea-pasquale changed the title Accepting circuit as initial state Accepting circuits as initial state Mar 6, 2023
@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change

Comparison is base (a72f6f7) 100.00% compared to head (3f6264c) 100.00%.

Additional details and impacted files
@@              Coverage Diff               @@
##           measurebasis      #817   +/-   ##
==============================================
  Coverage        100.00%   100.00%           
==============================================
  Files                97        97           
  Lines             12930     12953   +23     
==============================================
+ Hits              12930     12953   +23     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/qibo/models/circuit.py 100.00% <ø> (ø)
src/qibo/backends/numpy.py 100.00% <100.00%> (ø)
src/qibo/tests/test_models_circuit_execution.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

initial state for circuit with density_matrix {circuit.density_matrix}.""",
)
else:
return self.execute_circuit(initial_state + circuit, None, nshots)
Copy link
Member

Choose a reason for hiding this comment

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

Could you please update the documentation with this sum operation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the docstring. Thanks for pointing this out.

@andrea-pasquale andrea-pasquale changed the title Accepting circuits as initial state Accepting circuits as initial states Mar 6, 2023
Copy link
Member

@stavros11 stavros11 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 implementing this, looks good.

I was thinking that if you delegate the check if initial state is circuit in Circuit.execute we would not have to update both the numpy and qibolab backends. However, passing a circuit initial state would not work if one tries to use backend.execute_circuit(...) directly and this would create an assymetry. I think your solution is better.

@@ -331,6 +331,16 @@ def thermal_error_density_matrix(self, gate, state, nqubits):
def execute_circuit(
self, circuit, initial_state=None, nshots=None, return_array=False
):
if isinstance(initial_state, type(circuit)):
if not initial_state.density_matrix == circuit.density_matrix:
Copy link
Member

Choose a reason for hiding this comment

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

Looking at Circuit.__init__ another attribute that initial_state and circuit may be different is accelerators. I find it highly unlikely that someone will try to use this feature with multigpu, but maybe for consistency we could add the same check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this. I've just added a check also for accelerators.

@scarrazza scarrazza merged commit caf9c42 into measurebasis Mar 7, 2023
@scarrazza scarrazza deleted the circuit_as_initial_state branch March 14, 2023 17:33
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.

3 participants