Skip to content

Add problem info to the NL sampler result #565

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

Conversation

randomir
Copy link
Member

@randomir randomir commented Apr 4, 2025

Close #528.

Opted for the second option from #528, renaming timing to info and adding general problem info there, together with timing info and warnings.

The info field returned in LeapHybridNLSampler.SampleResult named tuple is a dict, now consistent with SampleSet.info, e.g.:

{
    "timing": {...},
    "warnings": [...],
    "problem_id": "...",
    "problem_label": "...",
    "problem_data_id": "..."
}

This change is technically breaking backwards-compatibility, but given that:

  • the result type from LeapHybridNLSampler.sample() hasn't been documented in full (name of the timing field is not specified),
  • no example either in docs, dwave-examples or elsewhere I could find actually uses the result,

I think we should be OK making this change.

randomir added 2 commits April 4, 2025 08:43
`info` field is a dict that includes timing info, and warnings pulled
out of the timing.
@randomir randomir requested a review from arcondello April 4, 2025 19:11
Copy link

codecov bot commented Apr 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.08%. Comparing base (47c368e) to head (739a432).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #565      +/-   ##
==========================================
- Coverage   88.88%   86.08%   -2.81%     
==========================================
  Files          21       21              
  Lines        1809     1818       +9     
==========================================
- Hits         1608     1565      -43     
- Misses        201      253      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@randomir randomir requested review from thisac and JoelPasvolsky April 4, 2025 19:14
@randomir randomir marked this pull request as draft April 4, 2025 19:15
Copy link
Contributor

@JoelPasvolsky JoelPasvolsky left a comment

Choose a reason for hiding this comment

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

would be nice if this repo had release notes for such updates

@randomir randomir marked this pull request as ready for review April 8, 2025 16:47
Copy link
Contributor

@thisac thisac left a comment

Choose a reason for hiding this comment

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

LGTM

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!

@@ -918,7 +918,7 @@ def parameters(self) -> Dict[str, List[str]]:

class SampleResult(NamedTuple):
model: dwave.optimization.Model
timing: dict
info: dict
Copy link
Member

Choose a reason for hiding this comment

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

I suppose we could say

    info: dict[str, typing.Any]

might be overkill but 🤷

Even more overkill would be to use typing.Literal.

Copy link
Member Author

Choose a reason for hiding this comment

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

And how about this? 😃

Copy link
Member

Choose a reason for hiding this comment

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

So annotated!

@randomir randomir merged commit 97720e7 into dwavesystems:master Apr 11, 2025
20 checks passed
@randomir randomir deleted the feature/add-problem-info-to-nl-sampler-result/issue-528 branch April 11, 2025 12:26
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.

Expose submitted problem ID from LeapHybridNLSampler
4 participants