refactor!: un-export six types that no caller could reach
`TimeSlice`, `EventKind`, `KeyTable`, `CompetitorStore`, `Competitor` and the `storage` module were all public and none was obtainable from a `History` — `time_slices`, `agents` and `keys` are all private or `pub(crate)`. `TimeSlice` was the worst: `new`, `add_events`, `iteration`, `get_composition` and `get_results` were `pub` on a type you could only build standalone and never feed back into anything. Their sole consumer outside `src/` was `benches/batch.rs`, so a benchmark was dictating six public types. It is rewritten against the public API: a single-slice history's `converge` calls exactly the same per-slice sweep, so capping at one iteration measures the same code path. `N01` had zero references in the entire repository, including inside the crate; removed. `N00` and `N_INF` are EP identities (`Add` and `Mul`) and are now `pub(crate)` — a user reaching for `N_INF` as "an unknown competitor's prior" would get an improper distribution whose `mu()` silently reports 0.0. Adds the accessors their absence forced people around, from #70: `competitors()`, `competitor_count()` and `event_count()` (`size` had no accessor at all). Answering "who is best" previously meant materialising every competitor's full smoothed curve to read the last point of each. `KeyTable::keys` now iterates the dense reverse table rather than the forward `HashMap`, so `competitors()` yields insertion order rather than per-process hash order — the same hazard as #62, caught before it could reach a caller building a standings table. Two `CompetitorStore` methods (`is_empty`, `iter_mut`) had no callers anywhere and are gone; four more are now `#[cfg(test)]`, which is what they always were in practice. Worth recording a mistake: I first deleted `get_composition`/`get_results` on the strength of a "never used" warning, and the build broke — the warning came from the plain-lib target, where `#[cfg(test)]` callers in history.rs are not compiled. A dead-code warning from one target is not evidence about the others. BREAKING CHANGE: `TimeSlice`, `EventKind`, `KeyTable`, `CompetitorStore`, `Competitor`, the `storage` module, `N01`, `N00` and `N_INF` are no longer public. Refs #73, #70 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
+45
-37
@@ -1,49 +1,57 @@
|
|||||||
|
//! One slice's event sweep.
|
||||||
|
//!
|
||||||
|
//! Written against the public API rather than against `TimeSlice` directly.
|
||||||
|
//! It used to reach for `TimeSlice`, `KeyTable`, `CompetitorStore`,
|
||||||
|
//! `Competitor` and `EventKind`, and was the *only* thing outside `src/`
|
||||||
|
//! that did — so a benchmark was dictating five public types that no test,
|
||||||
|
//! example or consumer could otherwise obtain.
|
||||||
|
//!
|
||||||
|
//! A single-slice history's `converge` calls exactly the same per-slice sweep,
|
||||||
|
//! so capping at one iteration measures the same code path.
|
||||||
|
|
||||||
use criterion::{Criterion, criterion_group, criterion_main};
|
use criterion::{Criterion, criterion_group, criterion_main};
|
||||||
|
use smallvec::smallvec;
|
||||||
use trueskill_tt::{
|
use trueskill_tt::{
|
||||||
BETA, Competitor, ConvergenceOptions, EventKind, GAMMA, KeyTable, MU, P_DRAW, Rating, SIGMA,
|
ConstantDrift, ConvergenceOptions, Event, History, Member, Outcome, Team,
|
||||||
TimeSlice, drift::ConstantDrift, gaussian::Gaussian, storage::CompetitorStore,
|
|
||||||
};
|
};
|
||||||
|
|
||||||
fn criterion_benchmark(criterion: &mut Criterion) {
|
fn criterion_benchmark(criterion: &mut Criterion) {
|
||||||
let mut index_map = KeyTable::new();
|
let build = || {
|
||||||
|
let mut h = History::builder()
|
||||||
|
.convergence(ConvergenceOptions {
|
||||||
|
max_iter: 1,
|
||||||
|
epsilon: 0.0,
|
||||||
|
alpha: 1.0,
|
||||||
|
})
|
||||||
|
.drift(ConstantDrift::new(0.0))
|
||||||
|
.build();
|
||||||
|
|
||||||
let a = index_map.get_or_create("a");
|
// 100 events, all at one time, so the history has a single slice.
|
||||||
let b = index_map.get_or_create("b");
|
let events: Vec<Event<i64, &'static str>> = (0..100)
|
||||||
let c = index_map.get_or_create("c");
|
.map(|_| Event {
|
||||||
|
time: 1,
|
||||||
|
teams: smallvec![
|
||||||
|
Team::with_members([Member::new("a")]),
|
||||||
|
Team::with_members([Member::new("b")]),
|
||||||
|
],
|
||||||
|
outcome: Outcome::winner(0, 2),
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
h.add_events(events).expect("fixture ingests");
|
||||||
|
h
|
||||||
|
};
|
||||||
|
|
||||||
let mut agents: CompetitorStore<i64, ConstantDrift> = CompetitorStore::new();
|
criterion.bench_function("slice_sweep_100_events", |b| {
|
||||||
|
b.iter_batched(
|
||||||
for agent in [a, b, c] {
|
build,
|
||||||
agents.insert(
|
|mut h| {
|
||||||
agent,
|
// `converge_partial`, not `converge`: one iteration is
|
||||||
Competitor {
|
// deliberately short of convergence and `converge` reports that
|
||||||
rating: Rating::new(
|
// as an error.
|
||||||
Gaussian::from_ms(MU, SIGMA),
|
let _ = h.converge_partial();
|
||||||
BETA,
|
|
||||||
ConstantDrift::new(GAMMA),
|
|
||||||
),
|
|
||||||
..Default::default()
|
|
||||||
},
|
},
|
||||||
|
criterion::BatchSize::SmallInput,
|
||||||
);
|
);
|
||||||
}
|
|
||||||
|
|
||||||
let mut composition = Vec::new();
|
|
||||||
let mut results = Vec::new();
|
|
||||||
let mut weights = Vec::new();
|
|
||||||
|
|
||||||
for _ in 0..100 {
|
|
||||||
composition.push(vec![vec![a], vec![b]]);
|
|
||||||
results.push(vec![1.0, 0.0]);
|
|
||||||
weights.push(vec![vec![1.0], vec![1.0]]);
|
|
||||||
}
|
|
||||||
|
|
||||||
let kinds = vec![EventKind::Ranked; composition.len()];
|
|
||||||
|
|
||||||
let mut time_slice = TimeSlice::new(1, P_DRAW, ConvergenceOptions::default());
|
|
||||||
time_slice.add_events(composition, Some(results), Some(weights), kinds, &agents);
|
|
||||||
|
|
||||||
criterion.bench_function("Batch::iteration", |b| {
|
|
||||||
b.iter(|| time_slice.iteration(0, &agents))
|
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -546,6 +546,39 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
|||||||
self.time_slices.len()
|
self.time_slices.len()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Every competitor the history knows, in the order they were first seen.
|
||||||
|
///
|
||||||
|
/// Includes competitors created by [`History::register`] that have not yet
|
||||||
|
/// appeared in an event.
|
||||||
|
///
|
||||||
|
/// Insertion order, not hash order: a `HashMap` walk differs between
|
||||||
|
/// processes, which would make anything built from this — a standings
|
||||||
|
/// table, a printed report — unreproducible.
|
||||||
|
///
|
||||||
|
/// ```
|
||||||
|
/// # use trueskill_tt::History;
|
||||||
|
/// let mut h = History::builder().build();
|
||||||
|
/// h.record_winner(&"alice", &"bob", 1)?;
|
||||||
|
/// let names: Vec<_> = h.competitors().copied().collect();
|
||||||
|
/// assert_eq!(names, ["alice", "bob"]);
|
||||||
|
/// # Ok::<(), trueskill_tt::InferenceError>(())
|
||||||
|
/// ```
|
||||||
|
pub fn competitors(&self) -> impl ExactSizeIterator<Item = &K> {
|
||||||
|
self.keys.keys()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// How many competitors the history knows.
|
||||||
|
#[must_use]
|
||||||
|
pub fn competitor_count(&self) -> usize {
|
||||||
|
self.keys.len()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// How many events have been ingested.
|
||||||
|
#[must_use]
|
||||||
|
pub fn event_count(&self) -> usize {
|
||||||
|
self.size
|
||||||
|
}
|
||||||
|
|
||||||
/// Learning curves for all competitors, keyed by their user-facing key.
|
/// Learning curves for all competitors, keyed by their user-facing key.
|
||||||
pub fn learning_curves(&self) -> HashMap<K, Vec<(T, Gaussian)>> {
|
pub fn learning_curves(&self) -> HashMap<K, Vec<(T, Gaussian)>> {
|
||||||
#[cfg(feature = "rayon")]
|
#[cfg(feature = "rayon")]
|
||||||
|
|||||||
+8
-7
@@ -60,19 +60,20 @@ where
|
|||||||
self.reverse.get(idx.0)
|
self.reverse.get(idx.0)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn keys(&self) -> impl Iterator<Item = &K> {
|
/// Every key, in the order they were first interned.
|
||||||
self.forward.keys()
|
///
|
||||||
|
/// Iterates the dense reverse table rather than the forward `HashMap`.
|
||||||
|
/// Rust seeds its default hasher per process, so a `HashMap` walk yields a
|
||||||
|
/// different order on every run — which is fine for membership but not for
|
||||||
|
/// anything a caller might sum, sort or print.
|
||||||
|
pub fn keys(&self) -> impl ExactSizeIterator<Item = &K> {
|
||||||
|
self.reverse.iter()
|
||||||
}
|
}
|
||||||
|
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn len(&self) -> usize {
|
pub fn len(&self) -> usize {
|
||||||
self.reverse.len()
|
self.reverse.len()
|
||||||
}
|
}
|
||||||
|
|
||||||
#[must_use]
|
|
||||||
pub fn is_empty(&self) -> bool {
|
|
||||||
self.reverse.is_empty()
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl<K> Default for KeyTable<K>
|
impl<K> Default for KeyTable<K>
|
||||||
|
|||||||
+3
-7
@@ -109,7 +109,6 @@ mod approx;
|
|||||||
pub(crate) mod arena;
|
pub(crate) mod arena;
|
||||||
mod time;
|
mod time;
|
||||||
mod time_slice;
|
mod time_slice;
|
||||||
pub use time_slice::{EventKind, TimeSlice};
|
|
||||||
mod acquisition;
|
mod acquisition;
|
||||||
mod color_group;
|
mod color_group;
|
||||||
mod competitor;
|
mod competitor;
|
||||||
@@ -130,10 +129,9 @@ mod outcome;
|
|||||||
mod predict;
|
mod predict;
|
||||||
pub(crate) mod quadrature;
|
pub(crate) mod quadrature;
|
||||||
mod rating;
|
mod rating;
|
||||||
pub mod storage;
|
pub(crate) mod storage;
|
||||||
|
|
||||||
pub use acquisition::expected_information_gain;
|
pub use acquisition::expected_information_gain;
|
||||||
pub use competitor::Competitor;
|
|
||||||
pub use convergence::{ConvergenceOptions, ConvergenceReport};
|
pub use convergence::{ConvergenceOptions, ConvergenceReport};
|
||||||
pub use drift::{ConstantDrift, Drift};
|
pub use drift::{ConstantDrift, Drift};
|
||||||
pub use error::{InferenceError, UnknownKeys};
|
pub use error::{InferenceError, UnknownKeys};
|
||||||
@@ -142,7 +140,6 @@ pub use event_builder::EventBuilder;
|
|||||||
pub use game::{Game, GameOptions, OwnedGame};
|
pub use game::{Game, GameOptions, OwnedGame};
|
||||||
pub use gaussian::Gaussian;
|
pub use gaussian::Gaussian;
|
||||||
pub use history::{History, HistoryBuilder, Joint};
|
pub use history::{History, HistoryBuilder, Joint};
|
||||||
pub use key_table::KeyTable;
|
|
||||||
use matrix::Matrix;
|
use matrix::Matrix;
|
||||||
pub use observer::{NullObserver, Observer};
|
pub use observer::{NullObserver, Observer};
|
||||||
pub use outcome::Outcome;
|
pub use outcome::Outcome;
|
||||||
@@ -226,9 +223,8 @@ const HALF_LINE_WINDOW: f64 = 10.0;
|
|||||||
const NARROW_WINDOW_RATIO: f64 = 2.0e4;
|
const NARROW_WINDOW_RATIO: f64 = 2.0e4;
|
||||||
const ASYMPTOTIC_MILLS_ALPHA: f64 = 100.0;
|
const ASYMPTOTIC_MILLS_ALPHA: f64 = 100.0;
|
||||||
|
|
||||||
pub const N01: Gaussian = Gaussian::from_ms(0.0, 1.0);
|
pub(crate) const N00: Gaussian = Gaussian::from_ms(0.0, 0.0);
|
||||||
pub const N00: Gaussian = Gaussian::from_ms(0.0, 0.0);
|
pub(crate) const N_INF: Gaussian = Gaussian::from_ms(0.0, f64::INFINITY);
|
||||||
pub const N_INF: Gaussian = Gaussian::from_ms(0.0, f64::INFINITY);
|
|
||||||
|
|
||||||
#[derive(Copy, Clone, Default, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
|
#[derive(Copy, Clone, Default, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
|
||||||
pub struct Index(usize);
|
pub struct Index(usize);
|
||||||
|
|||||||
@@ -56,16 +56,16 @@ impl<T: Time, D: Drift<T>> CompetitorStore<T, D> {
|
|||||||
self.get(idx).is_some()
|
self.get(idx).is_some()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Test-only: no code path in the crate needs a count.
|
||||||
|
#[cfg(test)]
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn len(&self) -> usize {
|
pub fn len(&self) -> usize {
|
||||||
self.n_present
|
self.n_present
|
||||||
}
|
}
|
||||||
|
|
||||||
#[must_use]
|
/// Test-only: iterating every competitor is an assertion helper, not part
|
||||||
pub fn is_empty(&self) -> bool {
|
/// of inference, which walks slices rather than the store.
|
||||||
self.n_present == 0
|
#[cfg(test)]
|
||||||
}
|
|
||||||
|
|
||||||
pub fn iter(&self) -> impl Iterator<Item = (Index, &Competitor<T, D>)> {
|
pub fn iter(&self) -> impl Iterator<Item = (Index, &Competitor<T, D>)> {
|
||||||
self.competitors
|
self.competitors
|
||||||
.iter()
|
.iter()
|
||||||
@@ -73,13 +73,6 @@ impl<T: Time, D: Drift<T>> CompetitorStore<T, D> {
|
|||||||
.filter_map(|(i, slot)| slot.as_ref().map(|a| (Index(i), a)))
|
.filter_map(|(i, slot)| slot.as_ref().map(|a| (Index(i), a)))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn iter_mut(&mut self) -> impl Iterator<Item = (Index, &mut Competitor<T, D>)> {
|
|
||||||
self.competitors
|
|
||||||
.iter_mut()
|
|
||||||
.enumerate()
|
|
||||||
.filter_map(|(i, slot)| slot.as_mut().map(|a| (Index(i), a)))
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn values_mut(&mut self) -> impl Iterator<Item = &mut Competitor<T, D>> {
|
pub fn values_mut(&mut self) -> impl Iterator<Item = &mut Competitor<T, D>> {
|
||||||
self.competitors.iter_mut().filter_map(|s| s.as_mut())
|
self.competitors.iter_mut().filter_map(|s| s.as_mut())
|
||||||
}
|
}
|
||||||
|
|||||||
+5
-1
@@ -760,6 +760,8 @@ impl<T: Time> TimeSlice<T> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Test-only: reads the slice's shape back for assertions.
|
||||||
|
#[cfg(test)]
|
||||||
pub fn get_composition(&self) -> Vec<Vec<Vec<Index>>> {
|
pub fn get_composition(&self) -> Vec<Vec<Vec<Index>>> {
|
||||||
self.events
|
self.events
|
||||||
.iter()
|
.iter()
|
||||||
@@ -773,6 +775,8 @@ impl<T: Time> TimeSlice<T> {
|
|||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Test-only: reads the slice's shape back for assertions.
|
||||||
|
#[cfg(test)]
|
||||||
pub fn get_results(&self) -> Vec<Vec<f64>> {
|
pub fn get_results(&self) -> Vec<Vec<f64>> {
|
||||||
self.events
|
self.events
|
||||||
.iter()
|
.iter()
|
||||||
@@ -887,7 +891,7 @@ mod tests {
|
|||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::{
|
use crate::{
|
||||||
KeyTable, competitor::Competitor, drift::ConstantDrift, rating::Rating,
|
competitor::Competitor, drift::ConstantDrift, key_table::KeyTable, rating::Rating,
|
||||||
storage::CompetitorStore,
|
storage::CompetitorStore,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user