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 submitted problem ID from LeapHybridNLSampler #528

Open
randomir opened this issue Aug 8, 2024 · 7 comments
Open

Expose submitted problem ID from LeapHybridNLSampler #528

randomir opened this issue Aug 8, 2024 · 7 comments
Labels
feature-request/enhancement New feature or request

Comments

@randomir
Copy link
Member

randomir commented Aug 8, 2024

Problem ID is available in SampleSet.info for other solver/samplers, but not in the LeapHybridNLSampler.SampleResult named tuple.

One option is to expand the SampleResult tuple with a third field, info, but that breaks backward compatibility.

Another option is to generalize the timing field, but that again breaks compatibility if the field is renamed.

So the only "safe" option is to tuck problem_id in the timing dict, but that's too ugly.

@randomir randomir added the feature-request/enhancement New feature or request label Aug 8, 2024
@rahmanshamit
Copy link

rahmanshamit commented Dec 9, 2024

Is the Problem ID referred in this issue, the problem_data_id from here in line 945?

problem_data_id = self.solver.upload_nlm(model, max_num_states=max_num_states).result()

If that's the case, one still slightly ugly but backwards compatible way might be to have a simple wrapper for the SampleResult class:

class SampleResult(NamedTuple):

The updated code would be:

class SampleResult(NamedTuple):
        model: "dwave.optimization.Model"
        timing: dict

"""  Wrapper for SampleResult to include additional metadata like Problem ID """
class ExtendedSampleResult:
      def __init__(self, sample_result, problem_id=None):
          self._sample_result = sample_result
          self.info = {"problem_id": problem_data_id} if problem_data_id else { }

      def __getattr__(self, item):
          """ Delegate access to the original SampleResult attributes """
          return getattr(self._sample_result, item)

and then slightly update the collect function:

return LeapHybridNLSampler.SampleResult(model, timing)

Right now it returns LeapHybridNLSampler.SampleResult(model, timing).

The update would need to be:

sample_result = LeapHybridNLSampler.SampleResult(model, timing)
return LeapHybridNLSampler.WrappedSampleResult(sample_result, problem_id=problem_data_id)

This method doesn't effect any past uses and introduces no changes to existing fields or variables.

@randomir
Copy link
Member Author

randomir commented Dec 9, 2024

Is the Problem ID referred in this issue, the problem_data_id from here in line 945?

No, it's the problem ID as returned by SAPI on sampling job submit. We refer to problem_data_id (problem data) when submitting the sampling job.

@rahmanshamit
Copy link

rahmanshamit commented Dec 9, 2024

I see, hope I'm not hogging up the comments of this issue - would we be able to do sampler.properties['problem_id'] and then pass that to the SampleResult wrapper from the previous comment to still maintain all current uses of SampleResult and its fields?

@randomir
Copy link
Member Author

No, problem_id is not a sampler/solver property. it's only available on the future returned by sample_nlm() on line 947:

future = self.solver.sample_nlm(problem_data_id, time_limit=time_limit, **kwargs)

To retrieve it, you can use .id, or safer, .wait_id(). So, you would want to modify collect():

def collect():
timing = future.timing
for msg in timing.get('warnings', []):
# note: no point using stacklevel, as this is a different thread
warnings.warn(msg, category=UserWarning)
return LeapHybridNLSampler.SampleResult(model, timing)

to look like:

    def collect():
        timing = future.timing
        problem_id = future.wait_id()
        ...
        # store problem_id in the result

@rahmanshamit
Copy link

rahmanshamit commented Dec 11, 2024

I see, thank you!

In that case, could we get the problem_id in collect as shown above and then rather than returning 'LeapHybridNLSampler.SampleResult(model, timing)', return using the SampleResult wrapper?
This way we still don't change any existing use of SampleResult, don't generalize timing or tuck it in the timing dict.

All code changes would look like this:

class SampleResult(NamedTuple):
        model: "dwave.optimization.Model"
        timing: dict

class ExtendedSampleResult:
    """A wrapper for SampleResult to include problem_id."""
    def __init__(self, sample_result, problem_id):
        self.model = sample_result.model
        self.timing = sample_result.timing
        self.problem_id = problem_id
def collect():
      timing = future.timing
      problem_id = future.wait_id()
      for msg in timing.get('warnings', []):
          # note: no point using stacklevel, as this is a different thread
          warnings.warn(msg, category=UserWarning)

      sample_result = LeapHybridNLSampler.SampleResult(model, timing)
      return ExtendedSampleResult(sample_result, problem_id)

@randomir
Copy link
Member Author

Unfortunately, a namedtuple (what SampleResult is) can't be extended that easily. Its __slots__ are set to empty, and we can't just add any attribute to it. Note that

model, timing = sample_result

is a valid assignment. That's why adding a third attribute would break backwards compatibility.

But looks like we'll have to break it in the end if we want to return problem metadata like the id, label, etc.

@rahmanshamit
Copy link

I see, that makes sense.
Thank you for the feedback! Unfortunately at the moment that might be our best option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants