-
Notifications
You must be signed in to change notification settings - Fork 79
Add Per-Parameter Convenience Kernel Override #904
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
Changes from all commits
3c5adb5
e592192
c75f410
35c29f2
a01f2b8
3074916
c3f7efd
2a86b07
937944d
28352d7
f74e4bc
a67f4aa
30fb565
c7862ad
42fc1c7
bfa8983
ae3c5c5
1d92b0c
6c7e3f3
bd66fdf
b372ba8
47aa5fb
ad1f650
1417922
1f1e764
75cda3d
f9ac6ba
f94fb72
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 |
|---|---|---|
| @@ -1,10 +1,13 @@ | ||
| """Composite kernels (that is, kernels composed of other kernels).""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import gc | ||
| from functools import reduce | ||
| from operator import add, mul | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| from attrs import define, field | ||
| from attrs import define, evolve, field | ||
| from attrs.converters import optional as optional_c | ||
| from attrs.validators import deep_iterable, gt, instance_of, min_len | ||
| from attrs.validators import optional as optional_v | ||
|
|
@@ -16,6 +19,9 @@ | |
| from baybe.utils.basic import to_tuple | ||
| from baybe.utils.validation import finite_float | ||
|
|
||
| if TYPE_CHECKING: | ||
| from baybe.searchspace.core import SearchSpace | ||
|
|
||
|
|
||
| @define(frozen=True) | ||
| class ScaleKernel(CompositeKernel): | ||
|
|
@@ -42,6 +48,17 @@ class ScaleKernel(CompositeKernel): | |
| If ``False``, the output scale is frozen at its initial value and excluded from | ||
| optimization.""" | ||
|
|
||
| @override | ||
| def _without_parameter( | ||
| self, name: str, searchspace: SearchSpace, / | ||
| ) -> Kernel | None: | ||
| stripped = self.base_kernel._without_parameter(name, searchspace) | ||
| return None if stripped is None else evolve(self, base_kernel=stripped) | ||
|
|
||
| @override | ||
| def _with_parameter(self, name: str, /) -> Kernel: | ||
| return evolve(self, base_kernel=self.base_kernel._with_parameter(name)) | ||
|
|
||
| @override | ||
| def to_gpytorch(self, *args, **kwargs): | ||
| import torch | ||
|
|
@@ -68,6 +85,13 @@ class AdditiveKernel(CompositeKernel): | |
| ) | ||
| """The individual kernels to be summed.""" | ||
|
|
||
| @override | ||
| def _with_parameter(self, name: str, /) -> Kernel: | ||
| return evolve( | ||
| self, | ||
| base_kernels=tuple(k._with_parameter(name) for k in self.base_kernels), | ||
| ) | ||
|
|
||
| @override | ||
| def to_gpytorch(self, *args, **kwargs): | ||
| return reduce(add, (k.to_gpytorch(*args, **kwargs) for k in self.base_kernels)) | ||
|
|
@@ -85,6 +109,13 @@ class ProductKernel(CompositeKernel): | |
| ) | ||
| """The individual kernels to be multiplied.""" | ||
|
|
||
| @override | ||
| def _with_parameter(self, name: str, /) -> Kernel: | ||
|
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. I was wondering, why we do not support |
||
| return evolve( | ||
| self, | ||
| base_kernels=tuple(k._with_parameter(name) for k in self.base_kernels), | ||
| ) | ||
|
|
||
| @override | ||
| def to_gpytorch(self, *args, **kwargs): | ||
| return reduce(mul, (k.to_gpytorch(*args, **kwargs) for k in self.base_kernels)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,17 +3,19 @@ | |
| from __future__ import annotations | ||
|
|
||
| import gc | ||
| import sys | ||
| from abc import ABC, abstractmethod | ||
| from functools import cached_property | ||
| from typing import TYPE_CHECKING, Any, ClassVar | ||
| from typing import TYPE_CHECKING, Any, ClassVar, TypeAlias | ||
|
|
||
| import attrs | ||
| import pandas as pd | ||
| from attrs import define, field | ||
| from attrs import Converter, define, field | ||
| from attrs.converters import optional as optional_c | ||
| from attrs.validators import instance_of, min_len | ||
| from typing_extensions import override | ||
|
|
||
| from baybe.kernels.base import Kernel | ||
| from baybe.parameters.enum import ParameterEncoding | ||
| from baybe.serialization import ( | ||
| SerialMixin, | ||
|
|
@@ -22,15 +24,93 @@ | |
| from baybe.utils.metadata import MeasurableMetadata, to_metadata | ||
|
|
||
| if TYPE_CHECKING: | ||
| from gpytorch.kernels import Kernel as GPyTorchKernel | ||
|
|
||
| from baybe.parameters.enum import _ParameterKind | ||
| from baybe.searchspace.continuous import SubspaceContinuous | ||
| from baybe.searchspace.core import SearchSpace | ||
| from baybe.searchspace.discrete import SubspaceDiscrete | ||
|
|
||
| KernelOverride: TypeAlias = Kernel | GPyTorchKernel | ||
| else: | ||
| KernelOverride: TypeAlias = Kernel | ||
|
|
||
| # TODO: Reactive slots in all classes once cached_property is supported: | ||
| # https://github.com/python-attrs/attrs/issues/164 | ||
|
|
||
|
|
||
| def _iter_basic_kernels(kernel: Kernel): | ||
|
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. No return type annotation? |
||
| """Iterate over the basic kernel leaves of a BayBE kernel.""" | ||
| from baybe.kernels.base import BasicKernel | ||
| from baybe.kernels.composite import AdditiveKernel, ProductKernel, ScaleKernel | ||
|
|
||
| if isinstance(kernel, BasicKernel): | ||
| yield kernel | ||
| elif isinstance(kernel, ScaleKernel): | ||
| yield from _iter_basic_kernels(kernel.base_kernel) | ||
| elif isinstance(kernel, (AdditiveKernel, ProductKernel)): | ||
|
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. Shouldn't we have an "else" for potential additional subclasses/for raising errors right now? Otherwise, high danger of silently ignoring stuff we might add in the future. |
||
| for sub in kernel.base_kernels: | ||
| yield from _iter_basic_kernels(sub) | ||
|
|
||
|
|
||
| def _to_kernel_override( | ||
| value: KernelOverride | None, instance: Parameter | ||
| ) -> KernelOverride | None: | ||
| """Validate a kernel override and scope BayBE kernels to their parameter. | ||
|
|
||
| Args: | ||
| value: The provided kernel override. | ||
| instance: The parameter the override belongs to. | ||
|
|
||
| Raises: | ||
| ValueError: If a BayBE kernel targets a different parameter or a GPyTorch | ||
| kernel specifies explicit active dimensions. | ||
| TypeError: If the object is neither a BayBE nor a GPyTorch kernel. | ||
|
|
||
| Returns: | ||
| The validated override, with BayBE kernels scoped to the parameter. | ||
| """ | ||
| if value is None: | ||
| return None | ||
|
|
||
| # BayBE kernels: every basic leaf must be unscoped or scoped to the owner. The | ||
| # kernel is then rebound to the owning parameter (dropping unspecified names). | ||
| if isinstance(value, Kernel): | ||
| if any( | ||
| leaf.parameter_names not in (None, (instance.name,)) | ||
| for leaf in _iter_basic_kernels(value) | ||
| ): | ||
| raise ValueError( | ||
| f"The kernel provided for the kernel override of " | ||
| f"'{instance.__class__.__name__}' may only act on the parameter " | ||
| f"itself. Its basic kernels must specify 'parameter_names' as " | ||
| f"``None`` or ({instance.name!r},)." | ||
| ) | ||
| return value._with_parameter(instance.name) | ||
|
|
||
| # GPyTorch kernels: no explicit active dimensions allowed anywhere in the tree. | ||
| if sys.modules.get("gpytorch") is not None: | ||
|
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. Is there actually a situation where somebody would ever use this without having |
||
| from gpytorch.kernels import Kernel as GPyTorchKernel | ||
|
|
||
| if isinstance(value, GPyTorchKernel): | ||
| if any( | ||
| k.active_dims is not None | ||
| for k in value.modules() | ||
| if isinstance(k, GPyTorchKernel) | ||
| ): | ||
| raise ValueError( | ||
| "The GPyTorch kernel provided for the kernel override must not " | ||
| "specify 'active_dims'." | ||
| ) | ||
| return value | ||
|
|
||
| raise TypeError( | ||
| f"The object provided for the kernel override of " | ||
| f"'{instance.__class__.__name__}' must be a BayBE or GPyTorch kernel. " | ||
| f"Got: {type(value)}" | ||
| ) | ||
|
|
||
|
|
||
| @define(frozen=True, slots=False) | ||
| class Parameter(ABC, SerialMixin): | ||
| """Abstract base class for all parameters. | ||
|
|
@@ -47,6 +127,13 @@ class Parameter(ABC, SerialMixin): | |
| name: str = field(validator=(instance_of(str), min_len(1))) | ||
| """The name of the parameter""" | ||
|
|
||
| kernel_override: KernelOverride | None = field( | ||
| default=None, | ||
| converter=Converter(_to_kernel_override, takes_self=True), # type: ignore[misc, call-overload] | ||
| kw_only=True, | ||
| ) | ||
| """An optional kernel replacing the overall kernel for this parameter.""" | ||
|
|
||
| metadata: MeasurableMetadata = field( | ||
| factory=MeasurableMetadata, | ||
| converter=lambda x: to_metadata(x, MeasurableMetadata), | ||
|
|
@@ -111,7 +198,14 @@ def is_equivalent(self, other: Parameter) -> bool: | |
| """ | ||
| if type(self) is not type(other): | ||
| return False | ||
| return attrs.evolve(self, name=other.name) == other | ||
| # The override is owner-scoped, so rebind it to the other parameter's name. | ||
| kernel_override = self.kernel_override | ||
| if isinstance(kernel_override, Kernel): | ||
|
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. What happens in the case of a GPyTorch Kernel?
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. Also, Claude claims that two "seperately instantiated, but structurally identical GPyTorch overrides always compare unequal", so please double-check |
||
| kernel_override = kernel_override._with_parameter(other.name) | ||
| return ( | ||
| attrs.evolve(self, name=other.name, kernel_override=kernel_override) | ||
| == other | ||
| ) | ||
|
|
||
| @abstractmethod | ||
| def summary(self) -> dict: | ||
|
|
||
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.
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.
To mimic the style of the
_without_parameter