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

Add problem labelling support #444

Merged
merged 6 commits into from Jan 13, 2021

Conversation

randomir
Copy link
Member

No description provided.

`label` kwarg has been added to all `sample_*` methods on all solvers.
We add `computation.Future.label` and `problem_label` to
`Future.sampleset.info` dict.
Copy link
Member

@arcondello arcondello left a comment

Choose a reason for hiding this comment

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

LGTM

dwave/cloud/computation.py Outdated Show resolved Hide resolved
@JoelPasvolsky
Copy link
Contributor

JoelPasvolsky commented Jan 12, 2021

I'm only partway through but as it's approved I'll ask quickly before it's merged, @randomir can you say something about your considerations on explicitly having label in the sampling functions' parameters as you have versus pulling it out of **params? Thanks!

@randomir
Copy link
Member Author

@JoelPasvolsky, I went with explicit because label is not a solver parameter (unlike **params). In the past, we added offset explicitly as well (which is an optional problem parameter). So, in each sample_* method we now actually have 3 categories of arguments: problem description, solver params and common/shared/sampling params (only label for now).

Originally, we thought to have label as a solver parameter, but to allow filtering by label, SSW decided to implement it level up, next to problem id, problem type, status, etc.

Alternatively, we could repurpose **params as **kwargs, and then split them lower down before the actual submit. Technically, that's trivial, but it's a conceptual change requiring docs update and additional explanation. So, this time I just went along with #2, even if that makes the code/docs more verbose.

@JoelPasvolsky
Copy link
Contributor

Thanks, @randomir, that makes sense. I guess the reason this caught my attention is that I feel it's good to have the minimum number of arguments to sampling functions to make those simple conceptually for users, so every explicit argument should carry its weight, and it's not clear to me that most users will often use labels (they're very useful for examples such as those in dwave-examples and animated demos), but it's another argument they need to learn when they're getting started.

@randomir
Copy link
Member Author

@JoelPasvolsky, I agree. FWIW, dimod samplers in dwave-system use kwargs, so we don't have to extract label as explicit argument there. Most users (especially new users) interface only with dwave-system samplers anyhow.

@randomir randomir merged commit 9f707b1 into dwavesystems:master Jan 13, 2021
@randomir randomir deleted the feature/problem-label branch January 13, 2021 15:50
randomir added a commit that referenced this pull request Jan 18, 2021
Changes since 0.8.2:

New Features
---

- All SAPI requests retried whenever possible, controlled with new
  `http_retry_*` Client options (#414)
- Introduced `SAPIError` base exception with SAPI error message and
  code, other exceptions reorganized in a backwards-compatible way
- Support for problem labels: `sample_*` methods accept optional `label`
  kwarg, and `Future.sampleset.info` contains `problem_label` (#444)
- Switched to CircleCI (#119)

Fixes
---

- Handle remote disconnects in an edge case (#229)
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.

None yet

3 participants