feat: HistoryBuilder::default_rating_for, a rule instead of a roll call
`register` states configuration for one competitor, which needs the key
set up front. A consumer ingesting an event stream generally does not
have it — and "every layout is static" is a rule, not a list. This makes
it one statement that cannot be forgotten on an ingestion path.
History::builder()
.default_rating_for(|key: &&str| {
key.starts_with("layout_")
.then(|| StartingPoint::new().drift_scale(0.0))
})
.build()
A fifth type parameter, defaulted to `NoRule`, so it costs a caller who
does not use one exactly nothing: `History<String>` still spells out.
Two deviations from #53, both because implementing it exposed something
the issue could not have known.
**A trait, not a bare `Fn` bound.** #53's option 1 was a raw
`R: Fn(&K) -> Option<Rating<T, D>>`. A closure's type cannot be written
down, and the motivating consumer holds its `History` in application
state — so it has to name the type in a struct field, and option 1 makes
that impossible. `RatingRule<K>` is implementable on a named type;
`tests/rating_rule.rs` has the struct-field case that would not have
compiled otherwise. `default_rating_for` still takes a closure for the
common case, via `FnRule`.
**The rule returns a `StartingPoint`, not a `Rating`.** A `Rating` also
carries `beta` and the drift model, which describe the *history* rather
than one competitor — a rule that could vary them would be describing a
different model per competitor. What the create branch actually applies
is the prior and the drift scale, the same pair a `Member` may carry, so
that is what the rule supplies. It also keeps `RatingRule<K>` free of
`T` and `D`: with `Rating<T, D>` in the signature, `drift` and
`time_type` stop compiling after a rule is set, because
`R: RatingRule<K, T, D>` does not imply `R: RatingRule<K, T, D2>`.
**Precedence, which #53 left open: explicit beats the rule, field by
field.** The alternative — `ConflictingCompetitorConfig` — would make a
single exceptional competitor incompatible with having any rule at all.
Two *explicit* declarations that disagree stay an error, because neither
is more specific than the other, and a test pins that they still do.
`key_type` resets the rule to `NoRule`: a `RatingRule<K>` cannot answer
questions about `K2`.
Every test carries a control, and one of them corrected me. I first
asserted that a non-matching competitor's *posterior* was untouched.
It is not, and should not be: alice plays the pinned layout, and what
she learns from beating it depends on how sure the model is about it.
The control is her configuration.
Closes #53.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
+126
-23
@@ -17,6 +17,7 @@ use crate::{
|
||||
observer::{NullObserver, Observer},
|
||||
predict::Prediction,
|
||||
rating::Rating,
|
||||
rating_rule::{FnRule, NoRule, RatingRule, StartingPoint},
|
||||
sort_time,
|
||||
storage::CompetitorStore,
|
||||
time::Time,
|
||||
@@ -32,7 +33,7 @@ use crate::{
|
||||
/// an unknown key. None of them can be changed after `build`, because they
|
||||
/// define the model the fit is of.
|
||||
///
|
||||
/// Parameterised as [`History`] is, `HistoryBuilder<K, T, D, O>`, with the
|
||||
/// Parameterised as [`History`] is, `HistoryBuilder<K, T, D, O, R>`, with the
|
||||
/// same defaults.
|
||||
///
|
||||
/// Two of the setters change the builder's *type* rather than a field —
|
||||
@@ -47,6 +48,7 @@ pub struct HistoryBuilder<
|
||||
T: Time = i64,
|
||||
D: Drift<T> = ConstantDrift,
|
||||
O: Observer<T> = NullObserver,
|
||||
R: RatingRule<K> = NoRule,
|
||||
> {
|
||||
mu: f64,
|
||||
sigma: f64,
|
||||
@@ -57,11 +59,14 @@ pub struct HistoryBuilder<
|
||||
convergence: ConvergenceOptions,
|
||||
observer: O,
|
||||
unknown_keys: crate::UnknownKeys,
|
||||
rule: R,
|
||||
_time: PhantomData<T>,
|
||||
_key: PhantomData<K>,
|
||||
}
|
||||
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<K, T, D, O> {
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone, R: RatingRule<K>>
|
||||
HistoryBuilder<K, T, D, O, R>
|
||||
{
|
||||
/// Prior mean skill.
|
||||
///
|
||||
/// # Panics
|
||||
@@ -153,9 +158,10 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
/// implementation. `converge` checks the variance each competitor actually
|
||||
/// accumulates and reports `InvalidParameter` if it is negative or
|
||||
/// non-finite.
|
||||
pub fn drift<D2: Drift<T>>(self, drift: D2) -> HistoryBuilder<K, T, D2, O> {
|
||||
pub fn drift<D2: Drift<T>>(self, drift: D2) -> HistoryBuilder<K, T, D2, O, R> {
|
||||
HistoryBuilder {
|
||||
drift,
|
||||
rule: self.rule,
|
||||
mu: self.mu,
|
||||
sigma: self.sigma,
|
||||
beta: self.beta,
|
||||
@@ -252,13 +258,14 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
/// assert!(h.current_skill(&"alice").unwrap().mu() > 0.0);
|
||||
/// # Ok::<(), trueskill_tt::InferenceError>(())
|
||||
/// ```
|
||||
pub fn time_type<T2>(self) -> HistoryBuilder<K, T2, D, O>
|
||||
pub fn time_type<T2>(self) -> HistoryBuilder<K, T2, D, O, R>
|
||||
where
|
||||
T2: Time,
|
||||
D: Drift<T2>,
|
||||
O: Observer<T2>,
|
||||
{
|
||||
HistoryBuilder {
|
||||
rule: self.rule,
|
||||
mu: self.mu,
|
||||
sigma: self.sigma,
|
||||
beta: self.beta,
|
||||
@@ -286,8 +293,79 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
/// h.record_winner(&"alice".to_string(), &"bob".to_string(), 1)?;
|
||||
/// # Ok::<(), trueskill_tt::InferenceError>(())
|
||||
/// ```
|
||||
pub fn key_type<K2: Eq + Hash + Clone>(self) -> HistoryBuilder<K2, T, D, O> {
|
||||
pub fn key_type<K2: Eq + Hash + Clone>(self) -> HistoryBuilder<K2, T, D, O, NoRule> {
|
||||
HistoryBuilder {
|
||||
// A `RatingRule<K>` cannot answer questions about `K2`, so
|
||||
// changing the key type drops it. Set the key type first.
|
||||
rule: NoRule,
|
||||
mu: self.mu,
|
||||
sigma: self.sigma,
|
||||
beta: self.beta,
|
||||
drift: self.drift,
|
||||
p_draw: self.p_draw,
|
||||
score_sigma: self.score_sigma,
|
||||
convergence: self.convergence,
|
||||
observer: self.observer,
|
||||
unknown_keys: self.unknown_keys,
|
||||
_time: PhantomData,
|
||||
_key: PhantomData,
|
||||
}
|
||||
}
|
||||
|
||||
/// Supply a rule that configures competitors the history has not seen.
|
||||
///
|
||||
/// `register` states configuration for one competitor at a time, which
|
||||
/// needs the key set up front. This states it for a *class* — "every
|
||||
/// layout is static" — as one statement that cannot be forgotten on an
|
||||
/// ingestion path.
|
||||
///
|
||||
/// Consulted once per competitor, at creation. Explicit configuration from
|
||||
/// `register` or a [`Member`](crate::Member) overrides it field by field;
|
||||
/// see [`RatingRule`] for why the specific beats the general here while
|
||||
/// two explicit declarations that disagree stay an error.
|
||||
///
|
||||
/// Changes the builder's type — bind the result — and must come *after*
|
||||
/// [`key_type`](HistoryBuilder::key_type), since a `RatingRule<K>` cannot
|
||||
/// answer questions about a different key type.
|
||||
///
|
||||
/// ```
|
||||
/// # use trueskill_tt::{Gaussian, History, StartingPoint};
|
||||
/// let h = History::builder()
|
||||
/// .default_rating_for(|key: &&'static str| {
|
||||
/// key.starts_with("bot_")
|
||||
/// .then(|| StartingPoint::new().drift_scale(0.0))
|
||||
/// })
|
||||
/// .build();
|
||||
/// # let _ = h;
|
||||
/// ```
|
||||
pub fn default_rating_for<F>(self, rule: F) -> HistoryBuilder<K, T, D, O, FnRule<F>>
|
||||
where
|
||||
F: Fn(&K) -> Option<StartingPoint>,
|
||||
{
|
||||
HistoryBuilder {
|
||||
rule: FnRule(rule),
|
||||
mu: self.mu,
|
||||
sigma: self.sigma,
|
||||
beta: self.beta,
|
||||
drift: self.drift,
|
||||
p_draw: self.p_draw,
|
||||
score_sigma: self.score_sigma,
|
||||
convergence: self.convergence,
|
||||
observer: self.observer,
|
||||
unknown_keys: self.unknown_keys,
|
||||
_time: PhantomData,
|
||||
_key: PhantomData,
|
||||
}
|
||||
}
|
||||
|
||||
/// Supply a rule as a named type implementing [`RatingRule`].
|
||||
///
|
||||
/// The counterpart of [`default_rating_for`](HistoryBuilder::default_rating_for)
|
||||
/// for when the resulting `History<..>` has to be written down in a struct
|
||||
/// field, which a closure's type makes impossible.
|
||||
pub fn rating_rule<R2: RatingRule<K>>(self, rule: R2) -> HistoryBuilder<K, T, D, O, R2> {
|
||||
HistoryBuilder {
|
||||
rule,
|
||||
mu: self.mu,
|
||||
sigma: self.sigma,
|
||||
beta: self.beta,
|
||||
@@ -308,8 +386,9 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
/// observer by value; to keep a handle on one that accumulates state, pass
|
||||
/// an `Arc` and keep a clone, or read it back with
|
||||
/// [`History::observer`] / [`History::into_observer`].
|
||||
pub fn observer<O2: Observer<T>>(self, observer: O2) -> HistoryBuilder<K, T, D, O2> {
|
||||
pub fn observer<O2: Observer<T>>(self, observer: O2) -> HistoryBuilder<K, T, D, O2, R> {
|
||||
HistoryBuilder {
|
||||
rule: self.rule,
|
||||
mu: self.mu,
|
||||
sigma: self.sigma,
|
||||
beta: self.beta,
|
||||
@@ -327,8 +406,9 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
/// Finish configuring and produce an empty [`History`].
|
||||
///
|
||||
/// Every parameter was validated as it was set, so this cannot fail.
|
||||
pub fn build(self) -> History<K, T, D, O> {
|
||||
pub fn build(self) -> History<K, T, D, O, R> {
|
||||
History {
|
||||
rule: self.rule,
|
||||
size: 0,
|
||||
time_slices: Vec::new(),
|
||||
competitors: CompetitorStore::new(),
|
||||
@@ -357,6 +437,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
impl<T: Time, K: Eq + Hash + Clone> Default for HistoryBuilder<K, T, ConstantDrift, NullObserver> {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
rule: NoRule,
|
||||
mu: MU,
|
||||
sigma: SIGMA,
|
||||
beta: BETA,
|
||||
@@ -451,7 +532,7 @@ impl CompetitorConfig {
|
||||
///
|
||||
/// # Type parameters
|
||||
///
|
||||
/// `History<K, T, D, O>` — key type, time type, drift model, observer — all
|
||||
/// `History<K, T, D, O, R>` — key type, time type, drift model, observer — all
|
||||
/// defaulted, so `History` alone means `&'static str` keys on an `i64` time
|
||||
/// axis with [`ConstantDrift`] and no observer, and `History<String>` is the
|
||||
/// whole spelling for owned keys.
|
||||
@@ -465,6 +546,7 @@ pub struct History<
|
||||
T: Time = i64,
|
||||
D: Drift<T> = ConstantDrift,
|
||||
O: Observer<T> = NullObserver,
|
||||
R: RatingRule<K> = NoRule,
|
||||
> {
|
||||
size: usize,
|
||||
pub(crate) time_slices: Vec<TimeSlice<T>>,
|
||||
@@ -478,6 +560,8 @@ pub struct History<
|
||||
score_sigma: f64,
|
||||
convergence: ConvergenceOptions,
|
||||
observer: O,
|
||||
/// Supplies a starting point for competitors nobody declared explicitly.
|
||||
rule: R,
|
||||
unknown_keys: crate::UnknownKeys,
|
||||
/// Competitor configuration explicitly declared so far, by whichever route.
|
||||
///
|
||||
@@ -552,7 +636,9 @@ impl<T: Time, K: Eq + Hash + Clone> HistoryBuilder<K, T, ConstantDrift, NullObse
|
||||
}
|
||||
}
|
||||
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D, O> {
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone, R: RatingRule<K>>
|
||||
History<K, T, D, O, R>
|
||||
{
|
||||
/// Promote a key to its [`Index`], creating the entry if it is new.
|
||||
///
|
||||
/// Crate-internal since #73: interning reserves a storage slot and nothing
|
||||
@@ -567,7 +653,9 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D
|
||||
}
|
||||
}
|
||||
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D, O> {
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone, R: RatingRule<K>>
|
||||
History<K, T, D, O, R>
|
||||
{
|
||||
fn iteration(&mut self) -> (f64, f64) {
|
||||
let mut step = (0.0, 0.0);
|
||||
|
||||
@@ -791,10 +879,13 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D
|
||||
self.beta,
|
||||
self.drift,
|
||||
);
|
||||
if let Some(prior) = member.prior {
|
||||
// The rule first, then the explicit values over the top of it: the
|
||||
// specific beats the general, field by field. See `RatingRule`.
|
||||
let from_rule = self.rule.starting_point(&member.key).unwrap_or_default();
|
||||
if let Some(prior) = member.prior.or(from_rule.prior) {
|
||||
rating.prior = prior;
|
||||
}
|
||||
if let Some(scale) = member.drift_scale {
|
||||
if let Some(scale) = member.drift_scale.or(from_rule.drift_scale) {
|
||||
rating.drift_scale = scale;
|
||||
}
|
||||
|
||||
@@ -1617,7 +1708,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D
|
||||
///
|
||||
/// `JointUnavailable` if the history is empty, contains ranked events, or
|
||||
/// yields a precision matrix that is not positive-definite.
|
||||
pub fn joint(&self) -> Result<Joint<'_, K, T, D, O>, InferenceError> {
|
||||
pub fn joint(&self) -> Result<Joint<'_, K, T, D, O, R>, InferenceError> {
|
||||
if self.time_slices.is_empty() {
|
||||
return Err(InferenceError::JointUnavailable {
|
||||
reason: "the history has no events",
|
||||
@@ -2088,7 +2179,9 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D
|
||||
}
|
||||
}
|
||||
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D, O> {
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone, R: RatingRule<K>>
|
||||
History<K, T, D, O, R>
|
||||
{
|
||||
pub(crate) fn add_events_with_prior(
|
||||
&mut self,
|
||||
mut composition: Vec<Vec<Vec<Index>>>,
|
||||
@@ -2325,10 +2418,17 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D
|
||||
self.beta,
|
||||
self.drift,
|
||||
);
|
||||
if let Some(prior) = config.prior {
|
||||
// Same precedence as `register`: the rule supplies a starting
|
||||
// point, explicit configuration overrides it field by field.
|
||||
let from_rule = self
|
||||
.keys
|
||||
.key(*competitor)
|
||||
.and_then(|key| self.rule.starting_point(key))
|
||||
.unwrap_or_default();
|
||||
if let Some(prior) = config.prior.or(from_rule.prior) {
|
||||
rating.prior = prior;
|
||||
}
|
||||
if let Some(scale) = config.drift_scale {
|
||||
if let Some(scale) = config.drift_scale.or(from_rule.drift_scale) {
|
||||
rating.drift_scale = scale;
|
||||
}
|
||||
|
||||
@@ -2533,7 +2633,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D
|
||||
}
|
||||
|
||||
/// Start a fluent event builder for a single match at `time`.
|
||||
pub fn event(&mut self, time: T) -> crate::event_builder::EventBuilder<'_, T, D, O, K> {
|
||||
pub fn event(&mut self, time: T) -> crate::event_builder::EventBuilder<'_, T, D, O, K, R> {
|
||||
crate::event_builder::EventBuilder::new(self, time)
|
||||
}
|
||||
|
||||
@@ -2693,8 +2793,8 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<K, T, D
|
||||
///
|
||||
/// It exists at all because without it a consumer cannot `#[derive(Debug)]` on
|
||||
/// any struct holding a `History`, which is how both known consumers store it.
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> std::fmt::Debug
|
||||
for History<K, T, D, O>
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone, R: RatingRule<K>> std::fmt::Debug
|
||||
for History<K, T, D, O, R>
|
||||
{
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("History")
|
||||
@@ -2758,8 +2858,9 @@ pub struct Joint<
|
||||
T: Time = i64,
|
||||
D: Drift<T> = ConstantDrift,
|
||||
O: Observer<T> = NullObserver,
|
||||
R: RatingRule<K> = NoRule,
|
||||
> {
|
||||
history: &'h History<K, T, D, O>,
|
||||
history: &'h History<K, T, D, O, R>,
|
||||
cholesky: crate::joint::Cholesky,
|
||||
/// `(row, slice)` of each competitor's latest appearance.
|
||||
latest: HashMap<Index, (usize, usize)>,
|
||||
@@ -2771,8 +2872,8 @@ pub struct Joint<
|
||||
|
||||
/// Deliberately does not print the factorisation, which is `n^2` floats and
|
||||
/// would make a `{:?}` of a large joint unreadable and slow.
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> std::fmt::Debug
|
||||
for Joint<'_, K, T, D, O>
|
||||
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone, R: RatingRule<K>> std::fmt::Debug
|
||||
for Joint<'_, K, T, D, O, R>
|
||||
{
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("Joint")
|
||||
@@ -2781,7 +2882,9 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> std::fmt::Debug
|
||||
}
|
||||
}
|
||||
|
||||
impl<K: Eq + Hash + Clone, T: Time, D: Drift<T>, O: Observer<T>> Joint<'_, K, T, D, O> {
|
||||
impl<K: Eq + Hash + Clone, T: Time, D: Drift<T>, O: Observer<T>, R: RatingRule<K>>
|
||||
Joint<'_, K, T, D, O, R>
|
||||
{
|
||||
/// Number of variables in the joint: the history's appearances, after
|
||||
/// collapsing consecutive pairs a competitor does not drift between.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user