logaritmisk/trueskill-tt
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Activity

Accessors disagree on how they report an unknown key, and learning_curve cannot distinguish a typo from a registered competitor #70

New Issue
Closed
opened 2026-09-09 17:53:50 +00:00 by logaritmisk · 2 comments
logaritmisk commented 2026-09-09 17:53:50 +00:00
Owner
Copy Link
Copy Source
Method Unknown key
lookup (history.rs:382) None
rating (:613) None
current_skill (:622) None
learning_curve (:635) empty Vec
filtered_learning_curve (:680) empty Vec
posterior_of Err(UnknownKey)
log_evidence_for silently whole-history (see #66)

Four different answers to the same question.

The learning_curve case is worse than inconsistent — it is ambiguous

An empty Vec is also what a known competitor with no appearances returns, and History::register creates exactly that state by design. Measured:

learning_curve("typo")                    = []
learning_curve("registered_never_played") = []

A typo and a deliberately registered competitor are indistinguishable. That matters precisely because register was added to make configuration explicit — and the accessor for "how has this competitor moved" cannot tell you whether it has heard of them.

This is the crate's own stated failure class, from UnknownKey's doc:

Reported rather than skipped: dropping unknown keys turns a team of strangers into a confident-looking probability about nobody.

Fix

pub fn learning_curve<Q>(&self, key: &Q) -> Option<Vec<(T, Gaussian)>>;
pub fn filtered_learning_curve<Q>(&self, key: &Q) -> Option<Vec<(T, Gaussian)>>;

None = never heard of them. Some(vec![]) = known, no appearances. That also aligns the singular accessors on one convention: Option for "is this competitor known", errors reserved for the calls that compute something.

Breaks: both return types.

Related gaps in the same family

You cannot enumerate a history's competitors. There is no competitors(), keys(), len() or is_empty() on History. KeyTable::keys() exists and KeyTable is exported, but History exposes no way to obtain one — so KeyTable::keys/len/is_empty have zero callers in the entire repo. Answering "who is best", the first question a stats consumer asks, currently requires materialising every competitor's full smoothed curve just to read the last point of each:

let mut board: Vec<_> = h.learning_curves().iter()
    .map(|(k, c)| (*k, c.last().unwrap().1.mu())).collect();

The evidence matrix is 3/4 complete, and the missing cell is the one cross-validation needs:

log_evidence()                  smoothed, all keys
log_evidence_for(keys)          smoothed, key-restricted
filtered_log_evidence()         forward-only, all keys
filtered_log_evidence_for(..)   MISSING

log_evidence_for is documented as "useful for leave-one-out cross-validation" and filtered_log_evidence as "the right quantity for prequential scoring" — per-competitor prequential scoring is exactly the intersection, and it is the one combination you cannot express. The plumbing already takes both axes: log_evidence_internal(forward: bool, targets: &[Index]). Only the (true, targets) corner has no public caller.

Suggested additions:

pub fn competitors(&self) -> impl ExactSizeIterator<Item = &K> + '_;
pub fn competitor_count(&self) -> usize;
pub fn event_count(&self) -> usize;                       // `size` has no accessor
pub fn current_skills(&self) -> HashMap<K, Gaussian>;     // plural of current_skill
pub fn filtered_log_evidence_for<Q>(&self, keys: &[&Q]) -> f64;

The learning-curve 2×2 itself (learning_curves / learning_curve / filtered_*) is complete and worth keeping as the model the other families should follow.

Breaks: the two return types above; the rest is additive.

Found by an API audit, 2026-09-09; the ambiguity reproduced independently.

| Method | Unknown key | |---|---| | `lookup` (`history.rs:382`) | `None` | | `rating` (`:613`) | `None` | | `current_skill` (`:622`) | `None` | | `learning_curve` (`:635`) | **empty `Vec`** | | `filtered_learning_curve` (`:680`) | **empty `Vec`** | | `posterior_of` | `Err(UnknownKey)` | | `log_evidence_for` | silently whole-history (see #66) | Four different answers to the same question. ## The `learning_curve` case is worse than inconsistent — it is ambiguous An empty `Vec` is *also* what a **known** competitor with no appearances returns, and `History::register` creates exactly that state by design. Measured: ``` learning_curve("typo") = [] learning_curve("registered_never_played") = [] ``` A typo and a deliberately registered competitor are indistinguishable. That matters precisely because `register` was added to make configuration explicit — and the accessor for "how has this competitor moved" cannot tell you whether it has heard of them. This is the crate's own stated failure class, from `UnknownKey`'s doc: > Reported rather than skipped: dropping unknown keys turns a team of strangers into a confident-looking probability about nobody. ## Fix ```rust pub fn learning_curve<Q>(&self, key: &Q) -> Option<Vec<(T, Gaussian)>>; pub fn filtered_learning_curve<Q>(&self, key: &Q) -> Option<Vec<(T, Gaussian)>>; ``` `None` = never heard of them. `Some(vec![])` = known, no appearances. That also aligns the singular accessors on one convention: `Option` for "is this competitor known", errors reserved for the calls that compute something. **Breaks:** both return types. ## Related gaps in the same family **You cannot enumerate a history's competitors.** There is no `competitors()`, `keys()`, `len()` or `is_empty()` on `History`. `KeyTable::keys()` exists and `KeyTable` is exported, but `History` exposes no way to obtain one — so `KeyTable::keys`/`len`/`is_empty` have zero callers in the entire repo. Answering "who is best", the first question a stats consumer asks, currently requires materialising every competitor's full smoothed curve just to read the last point of each: ```rust let mut board: Vec<_> = h.learning_curves().iter() .map(|(k, c)| (*k, c.last().unwrap().1.mu())).collect(); ``` **The evidence matrix is 3/4 complete**, and the missing cell is the one cross-validation needs: ``` log_evidence() smoothed, all keys log_evidence_for(keys) smoothed, key-restricted filtered_log_evidence() forward-only, all keys filtered_log_evidence_for(..) MISSING ``` `log_evidence_for` is documented as *"useful for leave-one-out cross-validation"* and `filtered_log_evidence` as *"the right quantity for prequential scoring"* — per-competitor prequential scoring is exactly the intersection, and it is the one combination you cannot express. The plumbing already takes both axes: `log_evidence_internal(forward: bool, targets: &[Index])`. Only the `(true, targets)` corner has no public caller. Suggested additions: ```rust pub fn competitors(&self) -> impl ExactSizeIterator<Item = &K> + '_; pub fn competitor_count(&self) -> usize; pub fn event_count(&self) -> usize; // `size` has no accessor pub fn current_skills(&self) -> HashMap<K, Gaussian>; // plural of current_skill pub fn filtered_log_evidence_for<Q>(&self, keys: &[&Q]) -> f64; ``` The learning-curve 2×2 itself (`learning_curves` / `learning_curve` / `filtered_*`) is complete and worth keeping as the model the other families should follow. **Breaks:** the two return types above; the rest is additive. Found by an API audit, 2026-09-09; the ambiguity reproduced independently.
logaritmisk added the apibreakingbug labels 2026-09-09 17:58:04 +00:00
logaritmisk commented 2026-09-09 19:14:02 +00:00
Author
Owner
Copy Link
Copy Source

Done in e4a68ba (merged as 60fc3e9), for the two curve accessors.

learning_curve and filtered_learning_curve now return Option<Vec<(T, Gaussian)>>: None for a key the history has never interned, Some(vec![]) for a competitor that is registered but has no appearances yet. The two were previously the same empty Vec.

tests/honest_accessors.rs asserts both halves against a control (a key with four appearances), so the test cannot pass by everything collapsing to one answer. tests/properties.rs also needed the distinction — a generated schedule need not touch every key, and that is now an explicit None branch rather than an empty vector that read as a passing assertion.

log_evidence_for was the other member of this family and is covered by #66.

Done in e4a68ba (merged as 60fc3e9), for the two curve accessors. `learning_curve` and `filtered_learning_curve` now return `Option<Vec<(T, Gaussian)>>`: `None` for a key the history has never interned, `Some(vec![])` for a competitor that is registered but has no appearances yet. The two were previously the same empty `Vec`. `tests/honest_accessors.rs` asserts both halves against a control (a key with four appearances), so the test cannot pass by everything collapsing to one answer. `tests/properties.rs` also needed the distinction — a generated schedule need not touch every key, and that is now an explicit `None` branch rather than an empty vector that read as a passing assertion. `log_evidence_for` was the other member of this family and is covered by #66.
logaritmisk commented 2026-09-09 19:19:30 +00:00
Author
Owner
Copy Link
Copy Source

The additive half of this issue is now done too, in 86e1521 (merged as cc601c0).

  • competitors(), competitor_count(), event_count() — landed earlier in the api/cleanup work.
  • current_skills() -> HashMap<K, Gaussian> — the leaderboard case. A registered-but-unplayed competitor is absent from the map, matching current_skill returning None.
  • filtered_log_evidence_for — the missing corner.

One correction to this issue's analysis: the missing corner is not log_evidence_internal(true, targets). That path selects skill.forward as each event's prior, which is only a filtering quantity on a history that has never been converged — iteration alternates sweeps, so from the second iteration the forward message has already absorbed backward information. log_evidence_internal's own doc says so. The new accessor goes through filtered_pass, like filtered_log_evidence, with the restriction applied to which events are scored rather than which are run: a held-out score under the real history, not a score under a counterfactual one where nobody else played.

tests/evidence_matrix.rs pins all four corners as distinct finite log probabilities, and carries controls both ways — naming every competitor must recover the unrestricted value, and the restricted forward-only value must differ from the restricted smoothed one.

The additive half of this issue is now done too, in 86e1521 (merged as cc601c0). - `competitors()`, `competitor_count()`, `event_count()` — landed earlier in the `api/cleanup` work. - `current_skills() -> HashMap<K, Gaussian>` — the leaderboard case. A registered-but-unplayed competitor is absent from the map, matching `current_skill` returning `None`. - `filtered_log_evidence_for` — the missing corner. One correction to this issue's analysis: the missing corner is **not** `log_evidence_internal(true, targets)`. That path selects `skill.forward` as each event's prior, which is only a filtering quantity on a history that has never been converged — `iteration` alternates sweeps, so from the second iteration the forward message has already absorbed backward information. `log_evidence_internal`'s own doc says so. The new accessor goes through `filtered_pass`, like `filtered_log_evidence`, with the restriction applied to which events are *scored* rather than which are *run*: a held-out score under the real history, not a score under a counterfactual one where nobody else played. `tests/evidence_matrix.rs` pins all four corners as distinct finite log probabilities, and carries controls both ways — naming every competitor must recover the unrestricted value, and the restricted forward-only value must differ from the restricted smoothed one.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
api
Shape or ergonomics of the public surface
 blocked
Waiting on another issue or an external decision
 breaking
Resolving it changes the public API; wants a major bump
 bug
Behaviour is wrong, including silently wrong
 decision
Needs a call to be made, not code to be written
 docs
Documentation, including doc comments and the README
 enhancement
New capability the crate does not have
 infra
CI, release process, packaging
 numerics
Floating-point accuracy, cancellation, convergence maths
 performance
Speed or memory
 refactor
Internal cleanup with no intended behaviour change
 testing
Test coverage and test infrastructure
No labels api breaking bug
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logaritmisk
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#70