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 innermost_child sampler #1145

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pau557
Copy link
Contributor

@pau557 pau557 commented Mar 9, 2022

Allow composite samplers to expose the innermost child sampler, this can be used to gather properties. For example, one can get QPU properties from a composite made of several layers

dimod/core/composite.py Outdated Show resolved Hide resolved
dimod/core/composite.py Outdated Show resolved Hide resolved
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.

Shouldn't this new feature be release noted?

@@ -74,8 +75,12 @@ def child(self):
except IndexError:
raise RuntimeError("A Composite must have at least one child Sampler")

def innermost_child(self) -> Sampler:
"""Returns the inner-most child sampler"""
Copy link
Contributor

@JoelPasvolsky JoelPasvolsky Mar 9, 2022

Choose a reason for hiding this comment

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

Suggested change
"""Returns the inner-most child sampler"""
"""Return the innermost child sampler."""

Copy link
Member

Choose a reason for hiding this comment

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

Also shouldn't it be Return?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, updated, thanks

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.

Need to add a release note.

Probably should be added to the docs as well, see https://github.com/dwavesystems/dimod/blob/main/docs/reference/sampler_composites/api.rst

@@ -317,3 +318,6 @@ def remove_unknown_kwargs(self, **kwargs) -> typing.Dict[str, typing.Any]:
kwargs.pop(kw)

return kwargs

def innermost_child(self) -> Sampler:
Copy link
Member

Choose a reason for hiding this comment

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

needs a docstring.


def test_composed_sampler(self):
innermost_child = dimod.ExactSolver()
sampler = dimod.ClipComposite(dimod.ScaleComposite(innermost_child))
Copy link
Member

Choose a reason for hiding this comment

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

I would use TrackingComposite and StructureComposite since those are staying in dimod through #1127 so the tests stay valid.

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.

3 participants