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

Sinter custom sampler #735

Closed
wants to merge 9 commits into from

Conversation

inmzhang
Copy link
Contributor

Fixes #682

This issue is related to one of my projects, so I attempted a basic and quick solution. I followed the pattern of sinter.Decoder and the implementation between the two should be similar.

The sampler has been poorly tested, and there may still be some issues with backward compatibility. However, I would appreciate your initial thoughts, as I'm not sure if this aligns with your design for this issue.

Copy link
Collaborator

@Strilanc Strilanc left a comment

Choose a reason for hiding this comment

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

This looks pretty good! I'll try it out for a bit before approving since it's a big change.

Sorry I took so long to review it. I think the main missing thing is to document it by editing the README and the getting started notebook. You may want to add an example of using it to the getting started notebook as well.

raise NotImplementedError("compile_sampler_for_circuit")

@abc.abstractmethod
def sample_detectors_via_files(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Drop this method. The files thing was a mistep; no need to repeat it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does that mean I should replace the method with something as below and avoid files entirely?

@abc.abstractmethod
def sample_detectors_bit_packed(
    self,
    *,
    circuit: stim.Circuit,
    shots: int,
) -> Tuple[np.ndarray, np.ndarray]:

efficiency.
"""
@abc.abstractmethod
def sample_detectors_bit_packed(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think, instead of sampling detection events, it would be better to sample AnonTaskStats directly. This would allow for much more flexibility in what a sampler did. But then I'm not sure how the decoder exactly fits into the picture... is it an argument to this method? Bleh...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! Could you give a usecase where sampling AnnoTaskStats will help? I'm not sure how to settle down the interface yet since it will introduce strong coupling between Sampler and Decoder if I feed the decoder as an argument to the sampling method.

@Strilanc
Copy link
Collaborator

Sorry for not responding to this for awhile. I took some inspiration from this PR and started iterating on it, and ultimately ended up going in a different direction. Specifically, I added a sinter.Sampler class and allowed it to go almost anywhere a sinter.Decoder can currently go, with the understanding that the sampler will take full responsibility for generating samples and producing statistics instead of only handling the decoding. This made fewer changes to the API surface, and fixed a nasty annoyance where not every sampler is compatible with every decoder (or even needs a decoder) but sinter was still asking for a decoder.

Closing as obsoleted by #804

@Strilanc Strilanc closed this Jul 27, 2024
Strilanc added a commit that referenced this pull request Sep 10, 2024
…#804)

- Add `sinter.Sampler` and `sinter.CompiledSampler` classes
- They can go anywhere a Decoder would go, but they are responsible for
all parts of the sampling instead of only prediction
- Add a new default sampler `perfectionist`, which discards anything
with detection events and predicts the observables are not flipped
- Improved layout of the progress printouts when collect is running
- Sinter decoders can now flag that they want to discard shots by adding
an extra byte to the returned observable data, with 0 meaning keep and
not-0 meaning discard
- Change how `sinter collect` distributes work
- Workers are now distributed as widely as possible, instead of all on
one task
- Workers are now never switched between tasks until their current task
is done
- Add `sinter plot --point_label_func` argument for drawing text next to
data points
- Augment `sinter plot --group_func` to support dictionaries with
special keys controlling precise grouping behaviors
- If group_func returns a dict with a `"color"` key, all items with the
same `"color"` value are drawn with the same color
- If group_func returns a dict with a `"linestyle"` key, all items with
the same `"linestyle"` value are drawn with the same linestyle
- If group_func returns a dict with a `"marker"` key, all items with the
same `"marker"` value are drawn with the same marker
- If group_func returns a dict with a `"label"` key, this forces the
label shown in the legend
- If group_func returns a dict with an `"order"` key, this takes
priority for ordering the legend
- `sinter collect --processes` is no longer required (defaults to
`"auto"`)
- `sinter plot --show` is no longer required (defaults to showing,
unless `--out` is specified, unless `--show` is specified)
- Group some of sinter's code into private subpackages
- Show traditional error bars instead of a filled region for high/low
fit when only one data point is present
- Add `sinter plot --preprocess_stats_func`
- Add `sinter.TaskStats.with_edits`
- Add safety error when adding stats that have equal strong ids but
differing identifying information (json_metadata or decoder)

Some of the sampler design is adapted from @inmzhang's design in
#735

Fixes #774

Fixes #682

Fixes #392

---------

Co-authored-by: Matt McEwen <[email protected]>
Strilanc added a commit that referenced this pull request Sep 10, 2024
…#804)

- Add `sinter.Sampler` and `sinter.CompiledSampler` classes
- They can go anywhere a Decoder would go, but they are responsible for
all parts of the sampling instead of only prediction
- Add a new default sampler `perfectionist`, which discards anything
with detection events and predicts the observables are not flipped
- Improved layout of the progress printouts when collect is running
- Sinter decoders can now flag that they want to discard shots by adding
an extra byte to the returned observable data, with 0 meaning keep and
not-0 meaning discard
- Change how `sinter collect` distributes work
- Workers are now distributed as widely as possible, instead of all on
one task
- Workers are now never switched between tasks until their current task
is done
- Add `sinter plot --point_label_func` argument for drawing text next to
data points
- Augment `sinter plot --group_func` to support dictionaries with
special keys controlling precise grouping behaviors
- If group_func returns a dict with a `"color"` key, all items with the
same `"color"` value are drawn with the same color
- If group_func returns a dict with a `"linestyle"` key, all items with
the same `"linestyle"` value are drawn with the same linestyle
- If group_func returns a dict with a `"marker"` key, all items with the
same `"marker"` value are drawn with the same marker
- If group_func returns a dict with a `"label"` key, this forces the
label shown in the legend
- If group_func returns a dict with an `"order"` key, this takes
priority for ordering the legend
- `sinter collect --processes` is no longer required (defaults to
`"auto"`)
- `sinter plot --show` is no longer required (defaults to showing,
unless `--out` is specified, unless `--show` is specified)
- Group some of sinter's code into private subpackages
- Show traditional error bars instead of a filled region for high/low
fit when only one data point is present
- Add `sinter plot --preprocess_stats_func`
- Add `sinter.TaskStats.with_edits`
- Add safety error when adding stats that have equal strong ids but
differing identifying information (json_metadata or decoder)

Some of the sampler design is adapted from @inmzhang's design in
#735

Fixes #774

Fixes #682

Fixes #392

---------

Co-authored-by: Matt McEwen <[email protected]>
Strilanc added a commit that referenced this pull request Sep 10, 2024
…#804)

- Add `sinter.Sampler` and `sinter.CompiledSampler` classes
- They can go anywhere a Decoder would go, but they are responsible for
all parts of the sampling instead of only prediction
- Add a new default sampler `perfectionist`, which discards anything
with detection events and predicts the observables are not flipped
- Improved layout of the progress printouts when collect is running
- Sinter decoders can now flag that they want to discard shots by adding
an extra byte to the returned observable data, with 0 meaning keep and
not-0 meaning discard
- Change how `sinter collect` distributes work
- Workers are now distributed as widely as possible, instead of all on
one task
- Workers are now never switched between tasks until their current task
is done
- Add `sinter plot --point_label_func` argument for drawing text next to
data points
- Augment `sinter plot --group_func` to support dictionaries with
special keys controlling precise grouping behaviors
- If group_func returns a dict with a `"color"` key, all items with the
same `"color"` value are drawn with the same color
- If group_func returns a dict with a `"linestyle"` key, all items with
the same `"linestyle"` value are drawn with the same linestyle
- If group_func returns a dict with a `"marker"` key, all items with the
same `"marker"` value are drawn with the same marker
- If group_func returns a dict with a `"label"` key, this forces the
label shown in the legend
- If group_func returns a dict with an `"order"` key, this takes
priority for ordering the legend
- `sinter collect --processes` is no longer required (defaults to
`"auto"`)
- `sinter plot --show` is no longer required (defaults to showing,
unless `--out` is specified, unless `--show` is specified)
- Group some of sinter's code into private subpackages
- Show traditional error bars instead of a filled region for high/low
fit when only one data point is present
- Add `sinter plot --preprocess_stats_func`
- Add `sinter.TaskStats.with_edits`
- Add safety error when adding stats that have equal strong ids but
differing identifying information (json_metadata or decoder)

Some of the sampler design is adapted from @inmzhang's design in
#735

Fixes #774

Fixes #682

Fixes #392

---------

Co-authored-by: Matt McEwen <[email protected]>
Strilanc added a commit that referenced this pull request Sep 10, 2024
…#804)

- Add `sinter.Sampler` and `sinter.CompiledSampler` classes
- They can go anywhere a Decoder would go, but they are responsible for
all parts of the sampling instead of only prediction
- Add a new default sampler `perfectionist`, which discards anything
with detection events and predicts the observables are not flipped
- Improved layout of the progress printouts when collect is running
- Sinter decoders can now flag that they want to discard shots by adding
an extra byte to the returned observable data, with 0 meaning keep and
not-0 meaning discard
- Change how `sinter collect` distributes work
- Workers are now distributed as widely as possible, instead of all on
one task
- Workers are now never switched between tasks until their current task
is done
- Add `sinter plot --point_label_func` argument for drawing text next to
data points
- Augment `sinter plot --group_func` to support dictionaries with
special keys controlling precise grouping behaviors
- If group_func returns a dict with a `"color"` key, all items with the
same `"color"` value are drawn with the same color
- If group_func returns a dict with a `"linestyle"` key, all items with
the same `"linestyle"` value are drawn with the same linestyle
- If group_func returns a dict with a `"marker"` key, all items with the
same `"marker"` value are drawn with the same marker
- If group_func returns a dict with a `"label"` key, this forces the
label shown in the legend
- If group_func returns a dict with an `"order"` key, this takes
priority for ordering the legend
- `sinter collect --processes` is no longer required (defaults to
`"auto"`)
- `sinter plot --show` is no longer required (defaults to showing,
unless `--out` is specified, unless `--show` is specified)
- Group some of sinter's code into private subpackages
- Show traditional error bars instead of a filled region for high/low
fit when only one data point is present
- Add `sinter plot --preprocess_stats_func`
- Add `sinter.TaskStats.with_edits`
- Add safety error when adding stats that have equal strong ids but
differing identifying information (json_metadata or decoder)

Some of the sampler design is adapted from @inmzhang's design in
#735

Fixes #774

Fixes #682

Fixes #392

---------

Co-authored-by: Matt McEwen <[email protected]>
Strilanc added a commit that referenced this pull request Sep 10, 2024
…#804)

- Add `sinter.Sampler` and `sinter.CompiledSampler` classes
- They can go anywhere a Decoder would go, but they are responsible for
all parts of the sampling instead of only prediction
- Add a new default sampler `perfectionist`, which discards anything
with detection events and predicts the observables are not flipped
- Improved layout of the progress printouts when collect is running
- Sinter decoders can now flag that they want to discard shots by adding
an extra byte to the returned observable data, with 0 meaning keep and
not-0 meaning discard
- Change how `sinter collect` distributes work
- Workers are now distributed as widely as possible, instead of all on
one task
- Workers are now never switched between tasks until their current task
is done
- Add `sinter plot --point_label_func` argument for drawing text next to
data points
- Augment `sinter plot --group_func` to support dictionaries with
special keys controlling precise grouping behaviors
- If group_func returns a dict with a `"color"` key, all items with the
same `"color"` value are drawn with the same color
- If group_func returns a dict with a `"linestyle"` key, all items with
the same `"linestyle"` value are drawn with the same linestyle
- If group_func returns a dict with a `"marker"` key, all items with the
same `"marker"` value are drawn with the same marker
- If group_func returns a dict with a `"label"` key, this forces the
label shown in the legend
- If group_func returns a dict with an `"order"` key, this takes
priority for ordering the legend
- `sinter collect --processes` is no longer required (defaults to
`"auto"`)
- `sinter plot --show` is no longer required (defaults to showing,
unless `--out` is specified, unless `--show` is specified)
- Group some of sinter's code into private subpackages
- Show traditional error bars instead of a filled region for high/low
fit when only one data point is present
- Add `sinter plot --preprocess_stats_func`
- Add `sinter.TaskStats.with_edits`
- Add safety error when adding stats that have equal strong ids but
differing identifying information (json_metadata or decoder)

Some of the sampler design is adapted from @inmzhang's design in
#735

Fixes #774

Fixes #682

Fixes #392

---------

Co-authored-by: Matt McEwen <[email protected]>
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.

Sinter Custom Sampler
2 participants