-
Notifications
You must be signed in to change notification settings - Fork 6
chore: sampler serialization tests #250
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
base: main
Are you sure you want to change the base?
Changes from all commits
d2405fd
b39bb6c
64846b2
4d8b3b6
d569be8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,9 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| import copy | ||
| from abc import ABC, abstractmethod | ||
| from typing import TYPE_CHECKING | ||
| from typing import TYPE_CHECKING, Any, NoReturn, Self | ||
|
|
||
| import numpy as np | ||
|
|
||
|
|
@@ -15,6 +16,32 @@ | |
| from annbatch.types import LoadRequest | ||
|
|
||
|
|
||
| def _attr_equal(a: object, b: object) -> bool: | ||
| """Structural equality for a single value stored in a sampler's ``__dict__``. | ||
|
|
||
| Handles the container types samplers keep as state -- most importantly the | ||
| :class:`numpy.random.Generator`, whose equality must compare the live *bit | ||
| generator state* (not object identity) so a round-tripped sampler counts as | ||
| equal to its source. numpy arrays and pandas objects (which return | ||
| element-wise ``==``) are compared structurally, and nested samplers dispatch | ||
| back to :meth:`Sampler.__eq__`. | ||
| """ | ||
| if isinstance(a, np.random.Generator) or isinstance(b, np.random.Generator): | ||
| return ( | ||
| isinstance(a, np.random.Generator) | ||
| and isinstance(b, np.random.Generator) | ||
| and a.bit_generator.state == b.bit_generator.state | ||
| ) | ||
| if isinstance(a, Sampler) or isinstance(b, Sampler): | ||
| return a == b | ||
| if isinstance(a, np.ndarray) or isinstance(b, np.ndarray): | ||
| return isinstance(a, np.ndarray) and isinstance(b, np.ndarray) and bool(np.array_equal(a, b)) | ||
| # pandas DataFrame/Series/Index/Categorical all expose a structural `.equals` | ||
| if hasattr(a, "equals") and hasattr(b, "equals") and type(a) is type(b): | ||
| return bool(a.equals(b)) | ||
| return bool(a == b) | ||
|
|
||
|
|
||
| class Sampler(ABC): | ||
| """Base sampler class. | ||
|
|
||
|
|
@@ -24,6 +51,41 @@ class Sampler(ABC): | |
| _mask: slice = slice(0, None) | ||
| _rng: np.random.Generator | None = None | ||
|
|
||
| def __eq__(self, other: object) -> bool: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this got lost in translation, but I think this should just be implemented custom per-implementation (so you don't have to handle every case as above and what constitutes "equal" is clear for every individual |
||
| """Two samplers are equal iff they have the same type and the same state. | ||
|
|
||
| State includes the random number generator's *bit generator state*, so a | ||
| sampler equals a pickle/deepcopy round-trip of itself but not a fresh (or | ||
| differently advanced) sampler built from the same seed. | ||
| """ | ||
| if type(self) is not type(other): | ||
| return NotImplemented | ||
| if self.__dict__.keys() != other.__dict__.keys(): | ||
| return False | ||
| return all(_attr_equal(self.__dict__[key], other.__dict__[key]) for key in self.__dict__) | ||
|
|
||
| def __copy__(self) -> NoReturn: | ||
| """Refuse shallow copies -- they would share the rng with the original. | ||
|
|
||
| A shallow copy keeps the same :class:`numpy.random.Generator` object, so | ||
| advancing one sampler would advance the "copy" too, silently breaking the | ||
| independence a copy is meant to provide. Use :func:`copy.deepcopy` instead. | ||
| """ | ||
| raise TypeError( | ||
| f"{type(self).__name__} does not support shallow copying: a shallow copy would share the " | ||
| "random number generator with the original, so the two would not sample independently. " | ||
| "Use copy.deepcopy() instead." | ||
| ) | ||
|
|
||
| def __deepcopy__(self, memo: dict[int, Any]) -> Self: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing as |
||
| """Deep-copy every attribute into an independent sampler (rng included).""" | ||
| cls = type(self) | ||
| new = cls.__new__(cls) | ||
| memo[id(self)] = new | ||
| for key, value in self.__dict__.items(): | ||
| setattr(new, key, copy.deepcopy(value, memo)) | ||
| return new | ||
|
|
||
| @property | ||
| def mask(self) -> slice: | ||
| """The observation range this sampler operates on.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe you mean this? otherwise, comparing two different ExtensionArrays will crash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function was one of the reasons I suggested moving to a per-class model: #250 (comment) It will make comparisons of the constituent parts much clearer, at this risk of the occasional repeated line of code.