feat: add UnknownKeys::Prior, and explain why there is no Skip
#44's third ask was an opt-in mode so a caller with partially-known teams need not pre-filter. The requested shape was `Skip` — drop unknown members. Measured, that is the wrong mode to build. A team's performance is the *sum* of its members, so dropping one drops its variance too. On a two-member team with one unknown: SKIP (drop the member) : performance sigma 2.37 PRIOR (member at prior) : performance sigma 6.53 (2.76x wider) Skipping makes the model *more* certain because it knows *less*, which is backwards. `Prior` is also the answer the model already gives for a competitor it knows about but has no evidence for — measured, such a competitor sits at sigma 4.99 against the prior's 6.0 — so it corresponds to a state the model can actually be in. Skipping does not. So the enum is `Reject` (default, unchanged) and `Prior`, and it is `#[non_exhaustive]` in case a real use for skipping turns up later. Placed on `HistoryBuilder` rather than per-call. Neither consumer wants it to vary between queries: one scores thousands of candidate matchups in a loop, the other's headline feature is predicting a competitor nobody has faced. That makes it a property of how the model is being used, and keeps five prediction signatures unchanged. This also gives #48 the semantics it asked for — "I have never seen this competitor, here is the prior-informed answer" — which it needs for predicting a course nobody has played. `an_unknown_member_widens_its_team_rather_than_narrowing_it` pins the property that ruled `Skip` out, so a future convenience cannot quietly reintroduce it. Closes #44. Refs #48 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
@@ -195,10 +195,25 @@ stay available at any size:
|
||||
quadratic in team count.
|
||||
- `predict_ranking(teams, ranks)` — one specific finishing order.
|
||||
|
||||
Unknown keys are an error, not a silent omission: a team the history has never
|
||||
seen cannot produce a confident-looking probability. The error names the key, and
|
||||
every key must already be known — pre-filter with `lookup` or `current_skill` if
|
||||
your caller cannot guarantee that.
|
||||
Unknown keys are an error by default, not a silent omission: a team the history
|
||||
has never seen cannot produce a confident-looking probability. The error names
|
||||
the key, and every key must already be known — pre-filter with `lookup` or
|
||||
`current_skill` if your caller cannot guarantee that.
|
||||
|
||||
If predicting for competitors you have never seen is the point rather than a
|
||||
mistake, say so once:
|
||||
|
||||
```rust
|
||||
use trueskill_tt::{History, UnknownKeys};
|
||||
|
||||
let h = History::builder().unknown_keys(UnknownKeys::Prior).build();
|
||||
```
|
||||
|
||||
An unknown competitor is then answered from the configured prior, which is the
|
||||
honest reading — you have no evidence about them — and correctly *widens* a team
|
||||
that contains one. There is deliberately no "skip the member" mode: a team's
|
||||
performance is the sum of its members, so dropping one would make the model more
|
||||
certain because it knows less.
|
||||
|
||||
### Asking about one competitor
|
||||
|
||||
|
||||
@@ -1,5 +1,44 @@
|
||||
use std::fmt;
|
||||
|
||||
/// How a prediction should treat a key the history has never seen.
|
||||
///
|
||||
/// Configured once per history via
|
||||
/// [`HistoryBuilder::unknown_keys`](crate::HistoryBuilder::unknown_keys).
|
||||
/// Neither known consumer wants this to vary between queries — one predicts
|
||||
/// thousands of candidate matchups in a loop, the other's headline feature is
|
||||
/// predicting a competitor nobody has faced — so it is a property of how you
|
||||
/// intend to use the model rather than an argument on five call sites.
|
||||
///
|
||||
/// # There is deliberately no `Skip`
|
||||
///
|
||||
/// Dropping an unknown member is the obvious third option and it is wrong. A
|
||||
/// team's performance is the *sum* of its members, so removing one removes its
|
||||
/// variance too: measured on a two-member team with one unknown, skipping gives
|
||||
/// a performance sigma of 2.37 where treating the member as unknown gives 6.53.
|
||||
/// An unknown competitor would make the model *more* certain, which is
|
||||
/// backwards. `Prior` is also the answer the model already gives for a
|
||||
/// competitor it knows about but has no evidence for, so it corresponds to a
|
||||
/// state the model can actually be in; skipping does not.
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
|
||||
#[non_exhaustive]
|
||||
pub enum UnknownKeys {
|
||||
/// Reject the prediction with [`InferenceError::UnknownKey`].
|
||||
///
|
||||
/// The default, and the right one when every key is expected to be known:
|
||||
/// a team of strangers should not silently produce a confident-looking
|
||||
/// answer.
|
||||
#[default]
|
||||
Reject,
|
||||
/// Treat an unknown competitor as one sitting at the history's configured
|
||||
/// prior.
|
||||
///
|
||||
/// This is the honest Bayesian reading — a competitor you have never
|
||||
/// observed is exactly the prior — and it makes "predict a matchup
|
||||
/// involving someone new" a first-class question rather than something a
|
||||
/// caller fakes with a neutral constant.
|
||||
Prior,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
#[non_exhaustive]
|
||||
pub enum InferenceError {
|
||||
|
||||
+40
-9
@@ -33,6 +33,7 @@ pub struct HistoryBuilder<
|
||||
score_sigma: f64,
|
||||
convergence: ConvergenceOptions,
|
||||
observer: O,
|
||||
unknown_keys: crate::UnknownKeys,
|
||||
_time: PhantomData<T>,
|
||||
_key: PhantomData<K>,
|
||||
}
|
||||
@@ -63,6 +64,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
score_sigma: self.score_sigma,
|
||||
convergence: self.convergence,
|
||||
observer: self.observer,
|
||||
unknown_keys: self.unknown_keys,
|
||||
_time: self._time,
|
||||
_key: self._key,
|
||||
}
|
||||
@@ -100,6 +102,20 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
self
|
||||
}
|
||||
|
||||
/// How predictions treat a key the history has never seen.
|
||||
///
|
||||
/// Defaults to [`UnknownKeys::Reject`](crate::UnknownKeys::Reject), which
|
||||
/// errors. Set [`UnknownKeys::Prior`](crate::UnknownKeys::Prior) to have an
|
||||
/// unknown competitor answered from the configured prior instead, which
|
||||
/// makes "predict a matchup involving someone new" a first-class question.
|
||||
///
|
||||
/// This affects predictions only. Ingestion always creates a competitor for
|
||||
/// a key it has not seen, because that is what an event *is*.
|
||||
pub fn unknown_keys(mut self, unknown_keys: crate::UnknownKeys) -> Self {
|
||||
self.unknown_keys = unknown_keys;
|
||||
self
|
||||
}
|
||||
|
||||
/// Convergence tolerance, iteration cap, and EP damping.
|
||||
///
|
||||
/// # Panics
|
||||
@@ -132,6 +148,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
score_sigma: self.score_sigma,
|
||||
convergence: self.convergence,
|
||||
observer,
|
||||
unknown_keys: self.unknown_keys,
|
||||
_time: self._time,
|
||||
_key: self._key,
|
||||
}
|
||||
@@ -151,6 +168,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
||||
score_sigma: self.score_sigma,
|
||||
convergence: self.convergence,
|
||||
observer: self.observer,
|
||||
unknown_keys: self.unknown_keys,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -166,6 +184,7 @@ impl Default for HistoryBuilder<i64, ConstantDrift, NullObserver, &'static str>
|
||||
score_sigma: 1.0,
|
||||
convergence: ConvergenceOptions::default(),
|
||||
observer: NullObserver,
|
||||
unknown_keys: crate::UnknownKeys::default(),
|
||||
_time: PhantomData,
|
||||
_key: PhantomData,
|
||||
}
|
||||
@@ -233,6 +252,7 @@ pub struct History<
|
||||
score_sigma: f64,
|
||||
convergence: ConvergenceOptions,
|
||||
observer: O,
|
||||
unknown_keys: crate::UnknownKeys,
|
||||
}
|
||||
|
||||
impl Default for History<i64, ConstantDrift, NullObserver, &'static str> {
|
||||
@@ -261,6 +281,7 @@ impl<K: Eq + Hash + Clone> History<i64, ConstantDrift, NullObserver, K> {
|
||||
score_sigma: 1.0,
|
||||
convergence: ConvergenceOptions::default(),
|
||||
observer: NullObserver,
|
||||
unknown_keys: crate::UnknownKeys::default(),
|
||||
_time: PhantomData,
|
||||
_key: PhantomData,
|
||||
}
|
||||
@@ -626,19 +647,29 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
||||
|
||||
let mut members = Vec::with_capacity(team.len());
|
||||
for (member_idx, key) in team.iter().enumerate() {
|
||||
let unknown = InferenceError::UnknownKey {
|
||||
team: team_idx,
|
||||
member: member_idx,
|
||||
key: format!("{key:?}"),
|
||||
};
|
||||
let index = self.keys.get(*key).ok_or(unknown.clone())?;
|
||||
members.push(
|
||||
// A key can be missing two ways — never interned, or interned
|
||||
// with no recorded skill — and both mean the same thing to a
|
||||
// caller, so they take the same branch.
|
||||
let skill = self.keys.get(*key).and_then(|index| {
|
||||
self.time_slices
|
||||
.iter()
|
||||
.rev()
|
||||
.find_map(|ts| ts.skills.get(index).map(|s| s.posterior()))
|
||||
.ok_or(unknown)?,
|
||||
);
|
||||
});
|
||||
|
||||
members.push(match skill {
|
||||
Some(skill) => skill,
|
||||
None => match self.unknown_keys {
|
||||
crate::UnknownKeys::Prior => Gaussian::from_ms(self.mu, self.sigma),
|
||||
_ => {
|
||||
return Err(InferenceError::UnknownKey {
|
||||
team: team_idx,
|
||||
member: member_idx,
|
||||
key: format!("{key:?}"),
|
||||
});
|
||||
}
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
gathered.push(members);
|
||||
|
||||
+1
-1
@@ -137,7 +137,7 @@ pub use acquisition::expected_information_gain;
|
||||
pub use competitor::Competitor;
|
||||
pub use convergence::{ConvergenceOptions, ConvergenceReport};
|
||||
pub use drift::{ConstantDrift, Drift};
|
||||
pub use error::InferenceError;
|
||||
pub use error::{InferenceError, UnknownKeys};
|
||||
pub use event::{Event, Member, Team};
|
||||
pub use event_builder::EventBuilder;
|
||||
pub use game::{Game, GameOptions, OwnedGame};
|
||||
|
||||
@@ -337,3 +337,81 @@ fn unknown_key_names_the_key_it_could_not_find() {
|
||||
"Display should say what to do about it: {rendered}"
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// UnknownKeys policy
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
fn history_with_policy(names: &[&'static str], policy: trueskill_tt::UnknownKeys) -> History {
|
||||
let mut h = History::builder().unknown_keys(policy).build();
|
||||
for pair in names.windows(2) {
|
||||
h.record_winner(&pair[0], &pair[1], 1).unwrap();
|
||||
}
|
||||
let _ = h.converge().unwrap();
|
||||
h
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reject_is_the_default() {
|
||||
let h = history_with(&["a", "b"], 0.0);
|
||||
assert!(matches!(
|
||||
h.predict_outcome(&[&[&"a"], &[&"ghost"]]),
|
||||
Err(InferenceError::UnknownKey { .. })
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prior_answers_instead_of_erroring() {
|
||||
let h = history_with_policy(&["a", "b"], trueskill_tt::UnknownKeys::Prior);
|
||||
let p = h
|
||||
.predict_outcome(&[&[&"a"], &[&"ghost"]])
|
||||
.expect("Prior should answer rather than reject");
|
||||
assert!((p.total() - 1.0).abs() < 1e-6);
|
||||
}
|
||||
|
||||
/// Two competitors the model has never seen are genuinely a coin flip. The
|
||||
/// point is that this is now *derived* rather than a constant a caller
|
||||
/// substitutes after swallowing an error.
|
||||
#[test]
|
||||
fn two_unknown_competitors_are_an_honest_coin_flip() {
|
||||
let h = history_with_policy(&["a", "b"], trueskill_tt::UnknownKeys::Prior);
|
||||
let wins = h
|
||||
.predict_win_probabilities(&[&[&"nobody"], &[&"no_one"]])
|
||||
.unwrap();
|
||||
assert!((wins[0] - 0.5).abs() < 1e-9, "{wins:?}");
|
||||
assert!((wins[1] - 0.5).abs() < 1e-9, "{wins:?}");
|
||||
}
|
||||
|
||||
/// The property that rules out a `Skip` mode: an unknown member must make a
|
||||
/// team *less* certain, never more. Skipping would drop the member's variance
|
||||
/// from the sum and narrow the team, which is backwards.
|
||||
#[test]
|
||||
fn an_unknown_member_widens_its_team_rather_than_narrowing_it() {
|
||||
let h = history_with_policy(&["a", "b", "c"], trueskill_tt::UnknownKeys::Prior);
|
||||
|
||||
// "a" alone against "b" — then "a" plus an unknown partner against "b".
|
||||
let solo = h.predict_win_probabilities(&[&[&"a"], &[&"b"]]).unwrap();
|
||||
let with_unknown = h
|
||||
.predict_win_probabilities(&[&[&"a", &"stranger"], &[&"b"]])
|
||||
.unwrap();
|
||||
|
||||
// Adding an unknown partner pulls the outcome toward even, because the
|
||||
// team's performance spread grew.
|
||||
assert!(
|
||||
(with_unknown[0] - 0.5).abs() < (solo[0] - 0.5).abs(),
|
||||
"an unknown partner should make the result less certain: solo {solo:?}, \
|
||||
with unknown {with_unknown:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prior_reaches_every_prediction_entry_point() {
|
||||
let h = history_with_policy(&["a", "b"], trueskill_tt::UnknownKeys::Prior);
|
||||
let teams: &[&[&&str]] = &[&[&"a"], &[&"ghost"]];
|
||||
|
||||
assert!(h.predict_quality(teams).is_ok());
|
||||
assert!(h.predict_win_probabilities(teams).is_ok());
|
||||
assert!(h.predict_outcome(teams).is_ok());
|
||||
assert!(h.predict_ranking(teams, &[0, 1]).is_ok());
|
||||
assert!(h.expected_information_gain(teams).is_ok());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user