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.

Sign up for GitHub

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

Jump to bottom

Can we deprecate StateVectorStepResult.set_state()? #4830

Closed
daxfohl opened this issue Jan 12, 2022 · 2 comments · Fixed by #4906
Closed

Can we deprecate StateVectorStepResult.set_state()? #4830

daxfohl opened this issue Jan 12, 2022 · 2 comments · Fixed by #4906
Labels
area/simulation kind/design-issue A conversation around design triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add

Comments

@daxfohl
Copy link
Contributor

daxfohl commented Jan 12, 2022 •
edited

Is your design idea/issue related to a use case or problem? Please describe.

StateVectorStepResult.set_state() and DensityMatrixStepResult.set_state() allow replacing the running simulation state with a new one. This ends up requiring a fair amount of code overhead to maintain. But I'm not sure this if this is a useful function, as it seems a user could just start a new simulation on the remaining circuit and the existing state. I'm guessing this function is just an artifact of an old design. If so, I think we should remove it.

Describe your design idea/issue

Deprecate this function, take advantage of cleanups it allows (StateVectorStepResult would no longer have to contain a simulator. SimulatorBase.core_iterator should no longer have to set sim_state after yielding the step result).

Obviously if we decide to do #4771 (remove the StepResult classes completely), then this is no longer relevant.
@daxfohl daxfohl added the kind/design-issue A conversation around design label Jan 12, 2022
@viathor viathor added area/simulation triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jan 12, 2022
@daxfohl daxfohl changed the title Is there a use case for StateVectorStepResult.set_state() Can we deprecate StateVectorStepResult.set_state()? Jan 15, 2022
@tanujkhattar
Copy link
Collaborator

From cirq sync:

Looks like an artefact of old times and should be deprecated. Let's ask around and see if anyone is still using this.

cc @Strilanc @mpharrigan

@tanujkhattar tanujkhattar added triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jan 26, 2022
@mpharrigan
Copy link
Collaborator

I don't know anything about this

@daxfohl daxfohl mentioned this issue Jan 27, 2022
Merged
CirqBot pushed a commit that referenced this issue Jan 29, 2022
@daxfohl
Deprecate set_state_vector and set_density_matrix (#4906)
2dc428b 
It was accepted at cirq sync that these are vestigial functions from a time when we didn't have initial_state or custom gates or classical controls that could be used for this. Now that we have all these, we can remove this function.

Fixes #4830
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@daxfohl @rht
Deprecate set_state_vector and set_density_matrix (quantumlib#4906)
e5e9402 
It was accepted at cirq sync that these are vestigial functions from a time when we didn't have initial_state or custom gates or classical controls that could be used for this. Now that we have all these, we can remove this function.

Fixes quantumlib#4830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/simulation kind/design-issue A conversation around design triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deprecate set_state_vector and set_density_matrix daxfohl/Cirq
4 participants
@daxfohl @mpharrigan @tanujkhattar @viathor