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

Expose reformatted parameters #465

Closed
arcondello opened this issue Apr 29, 2021 · 2 comments · Fixed by #510
Closed

Expose reformatted parameters #465

arcondello opened this issue Apr 29, 2021 · 2 comments · Fixed by #510

Comments

@arcondello
Copy link
Member

The StructuredSolver._format_params method currently transforms the initial_state parameter from a dict to the format expected by the endpoint.

This translation, while useful for the user, makes it hard for other libraries (e.g. aws-braket-ocean-plugin) that are trying to mock the behavior of the DWaveSampler from dwave-system to do so.

It would be helpful to expose this logic in a way that can be used by other libraries. Some possible approaches:

  1. Change the _format_params method to a public class method, or even a static method. This would allow users/libaries to use the method without needing to authenticate with D-Wave.
  2. Move _format_parmas to the DWaveSampler level, and expose it as a public method. We would likely want to deprecate the method in the cloud-client.

I am inclined towards (2).

@JoelPasvolsky
Copy link
Contributor

2 makes sense to me given that the transformation is specific to QPU. Where would it live in 1? Also I think with 1 it would need renaming, but that may be true for 2 also

@randomir
Copy link
Member

Keeping this particular function in the cloud client makes sense to me. Why? Because it translates a nice state map into a list with 3's - very SAPI-specific, low-level and ugly.

This is related to #166 and #199.

I'd say, ideally, we don't use SAPI-specific data encoding anywhere on the client's API surface, and the translation happens internally.

arcondello added a commit to arcondello/dwave_micro_client that referenced this issue Jan 24, 2022
arcondello added a commit to arcondello/dwave_micro_client that referenced this issue Jan 24, 2022
arcondello added a commit to arcondello/dwave_micro_client that referenced this issue Jan 24, 2022
arcondello added a commit to arcondello/dwave_micro_client that referenced this issue Jan 24, 2022
arcondello added a commit to arcondello/dwave_micro_client that referenced this issue Jan 24, 2022
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 a pull request may close this issue.

3 participants