feat: add the missing trait impls and make #[must_use] consistent

Trait coverage (#76), all additive:

  History          Debug is still absent - see below
  HistoryBuilder   + Debug   (it derived Clone but not Debug)
  Rating           + PartialEq  (Gaussian had it; Rating is a Gaussian
                                 plus three scalars and had none)
  Event/Team/Member + PartialEq (input value types with no way to compare
                                 them, which made round-trip tests awkward)
  ConvergenceReport + PartialEq

`#[must_use]` (#67). The coverage had no rule: `filtered_log_evidence`
had it and `log_evidence` did not; `rating` had it and `current_skill`
did not; `Rating::with_drift_scale` had it and `Member::with_drift_scale`
did not.

Now on the types — `EventBuilder`, `HistoryBuilder`, `Prediction`,
`Gaussian`, `OwnedGame` — which covers most method returns at once, plus
the `History` accessors individually.

`EventBuilder` gets a message, because a dropped builder is the worst
case in the set: measured, `h.event(1).team(["x"]).team(["y"]).winner(0)`
without `.commit()` leaves `time_slices_len() == 0` and every skill
`None`, with no warning at all.

And `ConvergenceReport`'s `#[must_use]` moves off the TYPE onto
`converge_partial`, where its stated reason is true. It read "from
`converge_partial` this may describe a fit that stopped at max_iter" but
fired on `converge` too — where that is false, since `converge` returns
`Err(NotConverged)` in exactly that case. So the crate's own front-page
example warned, and every quickstart had to write `let _ =`. Verified
from a consumer crate: `h.converge()?;` now compiles clean.

Marking the types made eight method-level attributes redundant, which
clippy's `double_must_use` caught — that is the type-level marker doing
its job, and the eight are removed.

Refs #76, #67

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
2026-09-09 20:38:39 +02:00
co-authored by Claude Opus 5
parent 4472d98b56
commit a0c2f78aed
11 changed files with 27 additions and 24 deletions
+1 -3
View File
@@ -11,9 +11,7 @@
use criterion::{Criterion, criterion_group, criterion_main}; use criterion::{Criterion, criterion_group, criterion_main};
use smallvec::smallvec; use smallvec::smallvec;
use trueskill_tt::{ use trueskill_tt::{ConstantDrift, ConvergenceOptions, Event, History, Member, Outcome, Team};
ConstantDrift, ConvergenceOptions, Event, History, Member, Outcome, Team,
};
fn criterion_benchmark(criterion: &mut Criterion) { fn criterion_benchmark(criterion: &mut Criterion) {
let build = || { let build = || {
+1 -5
View File
@@ -68,11 +68,7 @@ impl Default for ConvergenceOptions {
/// [`InferenceError::NotConverged`](crate::InferenceError::NotConverged) there. /// [`InferenceError::NotConverged`](crate::InferenceError::NotConverged) there.
/// From [`History::converge_partial`](crate::History::converge_partial) it may /// From [`History::converge_partial`](crate::History::converge_partial) it may
/// not be, and `converged` is what says so. /// not be, and `converged` is what says so.
#[derive(Clone, Debug)] #[derive(Clone, Debug, PartialEq)]
#[must_use = "from `converge_partial` this may describe a fit that stopped at \
`max_iter`, which is wrong by a little rather than loudly \
broken — check `converged`, or bind it to `_` to say you have \
decided not to"]
pub struct ConvergenceReport { pub struct ConvergenceReport {
pub iterations: usize, pub iterations: usize,
pub final_step: (f64, f64), pub final_step: (f64, f64),
+3 -3
View File
@@ -11,7 +11,7 @@ use smallvec::SmallVec;
use crate::{gaussian::Gaussian, outcome::Outcome, time::Time}; use crate::{gaussian::Gaussian, outcome::Outcome, time::Time};
/// A single match at time `time` involving some number of teams. /// A single match at time `time` involving some number of teams.
#[derive(Clone, Debug)] #[derive(Clone, Debug, PartialEq)]
pub struct Event<T: Time, K> { pub struct Event<T: Time, K> {
pub time: T, pub time: T,
pub teams: SmallVec<[Team<K>; 4]>, pub teams: SmallVec<[Team<K>; 4]>,
@@ -19,7 +19,7 @@ pub struct Event<T: Time, K> {
} }
/// A team: list of members competing together. /// A team: list of members competing together.
#[derive(Clone, Debug)] #[derive(Clone, Debug, PartialEq)]
pub struct Team<K> { pub struct Team<K> {
pub members: SmallVec<[Member<K>; 4]>, pub members: SmallVec<[Member<K>; 4]>,
} }
@@ -61,7 +61,7 @@ impl<K> Default for Team<K> {
/// for one competitor within a single batch is /// for one competitor within a single batch is
/// `InferenceError::ConflictingCompetitorConfig`: events in a batch have no /// `InferenceError::ConflictingCompetitorConfig`: events in a batch have no
/// order, so there would be no well-defined winner. /// order, so there would be no well-defined winner.
#[derive(Clone, Debug)] #[derive(Clone, Debug, PartialEq)]
pub struct Member<K> { pub struct Member<K> {
pub key: K, pub key: K,
pub weight: f64, pub weight: f64,
+2
View File
@@ -9,6 +9,8 @@ use crate::{
time::Time, time::Time,
}; };
#[must_use = "an event is only recorded by `.commit()`; a dropped builder \
silently ingests nothing"]
pub struct EventBuilder<'h, T, D, O, K> pub struct EventBuilder<'h, T, D, O, K>
where where
T: Time, T: Time,
-1
View File
@@ -44,7 +44,6 @@ impl VarStore {
id id
} }
#[must_use]
pub fn get(&self, id: VarId) -> Gaussian { pub fn get(&self, id: VarId) -> Gaussian {
self.marginals[id.0 as usize] self.marginals[id.0 as usize]
} }
+1
View File
@@ -92,6 +92,7 @@ impl Default for GameOptions {
/// can be returned freely from public constructors. The inference inputs /// can be returned freely from public constructors. The inference inputs
/// themselves are not retained — nothing reads them back. /// themselves are not retained — nothing reads them back.
#[derive(Debug)] #[derive(Debug)]
#[must_use]
pub struct OwnedGame<T: Time, D: Drift<T>> { pub struct OwnedGame<T: Time, D: Drift<T>> {
teams: Vec<Vec<Rating<T, D>>>, teams: Vec<Vec<Rating<T, D>>>,
pub(crate) likelihoods: Vec<Vec<Gaussian>>, pub(crate) likelihoods: Vec<Vec<Gaussian>>,
+1 -2
View File
@@ -11,6 +11,7 @@ use crate::{MU, N_INF, SIGMA};
/// the stored fields with no `sqrt` or reciprocal in the hot path. `mu()` and /// the stored fields with no `sqrt` or reciprocal in the hot path. `mu()` and
/// `sigma()` are accessors computed on demand. /// `sigma()` are accessors computed on demand.
#[derive(Clone, Copy, PartialEq, Debug)] #[derive(Clone, Copy, PartialEq, Debug)]
#[must_use]
pub struct Gaussian { pub struct Gaussian {
pi: f64, pi: f64,
tau: f64, tau: f64,
@@ -44,7 +45,6 @@ impl Gaussian {
/// small truncated sigma and inference must not panic. It is worth knowing /// small truncated sigma and inference must not panic. It is worth knowing
/// that such a `Gaussian` is not equal to itself, so two identical /// that such a `Gaussian` is not equal to itself, so two identical
/// declarations of one can be reported as conflicting. /// declarations of one can be reported as conflicting.
#[must_use]
pub const fn from_ms(mu: f64, sigma: f64) -> Self { pub const fn from_ms(mu: f64, sigma: f64) -> Self {
// NaN is admitted on purpose. A broken fit legitimately produces a NaN // NaN is admitted on purpose. A broken fit legitimately produces a NaN
// sigma — `sqrt` of a negative truncated variance — and the design is // sigma — `sqrt` of a negative truncated variance — and the design is
@@ -243,7 +243,6 @@ impl Gaussian {
/// Used by within-game inference to stabilise oscillating fixed-point /// Used by within-game inference to stabilise oscillating fixed-point
/// loops on hard graphs. `alpha = 1.0` returns `new` exactly; /// loops on hard graphs. `alpha = 1.0` returns `new` exactly;
/// `alpha < 1.0` shrinks each per-step update. /// `alpha < 1.0` shrinks each per-step update.
#[must_use]
pub fn damp_natural(self, new: Gaussian, alpha: f64) -> Gaussian { pub fn damp_natural(self, new: Gaussian, alpha: f64) -> Gaussian {
Gaussian::from_natural( Gaussian::from_natural(
alpha * new.pi() + (1.0 - alpha) * self.pi(), alpha * new.pi() + (1.0 - alpha) * self.pi(),
+13 -5
View File
@@ -24,7 +24,8 @@ use crate::{
tuple_gt, tuple_max, tuple_gt, tuple_max,
}; };
#[derive(Clone)] #[derive(Clone, Debug)]
#[must_use = "a builder does nothing until `.build()`"]
pub struct HistoryBuilder< pub struct HistoryBuilder<
T: Time = i64, T: Time = i64,
D: Drift<T> = ConstantDrift, D: Drift<T> = ConstantDrift,
@@ -197,7 +198,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
/// assert!(h.current_skill(&"alice").unwrap().mu() > 0.0); /// assert!(h.current_skill(&"alice").unwrap().mu() > 0.0);
/// # Ok::<(), trueskill_tt::InferenceError>(()) /// # Ok::<(), trueskill_tt::InferenceError>(())
/// ``` /// ```
#[must_use]
pub fn time_type<T2>(self) -> HistoryBuilder<T2, D, O, K> pub fn time_type<T2>(self) -> HistoryBuilder<T2, D, O, K>
where where
T2: Time, T2: Time,
@@ -232,7 +232,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
/// h.record_winner(&"alice".to_string(), &"bob".to_string(), 1)?; /// h.record_winner(&"alice".to_string(), &"bob".to_string(), 1)?;
/// # Ok::<(), trueskill_tt::InferenceError>(()) /// # Ok::<(), trueskill_tt::InferenceError>(())
/// ``` /// ```
#[must_use]
pub fn key_type<K2: Eq + Hash + Clone>(self) -> HistoryBuilder<T, D, O, K2> { pub fn key_type<K2: Eq + Hash + Clone>(self) -> HistoryBuilder<T, D, O, K2> {
HistoryBuilder { HistoryBuilder {
mu: self.mu, mu: self.mu,
@@ -418,7 +417,6 @@ impl Default for History<i64, ConstantDrift, NullObserver, &'static str> {
} }
impl History<i64, ConstantDrift, NullObserver, &'static str> { impl History<i64, ConstantDrift, NullObserver, &'static str> {
#[must_use]
pub fn builder() -> HistoryBuilder<i64, ConstantDrift, NullObserver, &'static str> { pub fn builder() -> HistoryBuilder<i64, ConstantDrift, NullObserver, &'static str> {
HistoryBuilder::default() HistoryBuilder::default()
} }
@@ -440,7 +438,6 @@ impl<T: Time, K: Eq + Hash + Clone> HistoryBuilder<T, ConstantDrift, NullObserve
/// h.converge()?; /// h.converge()?;
/// # Ok::<(), trueskill_tt::InferenceError>(()) /// # Ok::<(), trueskill_tt::InferenceError>(())
/// ``` /// ```
#[must_use]
pub fn new() -> Self { pub fn new() -> Self {
Self::default() Self::default()
} }
@@ -455,6 +452,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
self.keys.get_or_create(key) self.keys.get_or_create(key)
} }
#[must_use]
pub fn lookup<Q>(&self, key: &Q) -> Option<Index> pub fn lookup<Q>(&self, key: &Q) -> Option<Index>
where where
K: Borrow<Q>, K: Borrow<Q>,
@@ -563,6 +561,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// assert_eq!(names, ["alice", "bob"]); /// assert_eq!(names, ["alice", "bob"]);
/// # Ok::<(), trueskill_tt::InferenceError>(()) /// # Ok::<(), trueskill_tt::InferenceError>(())
/// ``` /// ```
#[must_use]
pub fn competitors(&self) -> impl ExactSizeIterator<Item = &K> { pub fn competitors(&self) -> impl ExactSizeIterator<Item = &K> {
self.keys.keys() self.keys.keys()
} }
@@ -580,6 +579,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
} }
/// Learning curves for all competitors, keyed by their user-facing key. /// Learning curves for all competitors, keyed by their user-facing key.
#[must_use]
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")]
{ {
@@ -728,6 +728,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
self.agents.contains(idx).then(|| self.agents[idx].rating) self.agents.contains(idx).then(|| self.agents[idx].rating)
} }
#[must_use]
pub fn current_skill<Q>(&self, key: &Q) -> Option<Gaussian> pub fn current_skill<Q>(&self, key: &Q) -> Option<Gaussian>
where where
K: std::borrow::Borrow<Q>, K: std::borrow::Borrow<Q>,
@@ -741,6 +742,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
} }
/// Learning curve for a single key: (time, posterior) pairs in time order. /// Learning curve for a single key: (time, posterior) pairs in time order.
#[must_use]
pub fn learning_curve<Q>(&self, key: &Q) -> Vec<(T, Gaussian)> pub fn learning_curve<Q>(&self, key: &Q) -> Vec<(T, Gaussian)>
where where
K: std::borrow::Borrow<Q>, K: std::borrow::Borrow<Q>,
@@ -764,6 +766,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// Runs a full forward pass per call and caches nothing. This is the /// Runs a full forward pass per call and caches nothing. This is the
/// entry point for multi-key work — see `filtered_learning_curve` for /// entry point for multi-key work — see `filtered_learning_curve` for
/// why calling that once per key is far more expensive. /// why calling that once per key is far more expensive.
#[must_use]
pub fn filtered_learning_curves(&self) -> HashMap<K, Vec<(T, Gaussian)>> { pub fn filtered_learning_curves(&self) -> HashMap<K, Vec<(T, Gaussian)>> {
let mut data: HashMap<K, Vec<(T, Gaussian)>> = HashMap::new(); let mut data: HashMap<K, Vec<(T, Gaussian)>> = HashMap::new();
@@ -786,6 +789,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// discarding every posterior but the requested key's. N keys fetched /// discarding every posterior but the requested key's. N keys fetched
/// this way costs O(N * events); use `filtered_learning_curves` for /// this way costs O(N * events); use `filtered_learning_curves` for
/// multi-key work instead — it computes the same pass once. /// multi-key work instead — it computes the same pass once.
#[must_use]
pub fn filtered_learning_curve<Q>(&self, key: &Q) -> Vec<(T, Gaussian)> pub fn filtered_learning_curve<Q>(&self, key: &Q) -> Vec<(T, Gaussian)>
where where
K: Borrow<Q>, K: Borrow<Q>,
@@ -841,12 +845,14 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
} }
/// Total log-evidence across the history. /// Total log-evidence across the history.
#[must_use]
pub fn log_evidence(&self) -> f64 { pub fn log_evidence(&self) -> f64 {
self.log_evidence_internal(false, &[]) self.log_evidence_internal(false, &[])
} }
/// Log-evidence restricted to time slices containing at least one of the /// Log-evidence restricted to time slices containing at least one of the
/// given keys. Useful for leave-one-out cross-validation. /// given keys. Useful for leave-one-out cross-validation.
#[must_use]
pub fn log_evidence_for<Q>(&self, keys: &[&Q]) -> f64 pub fn log_evidence_for<Q>(&self, keys: &[&Q]) -> f64
where where
K: std::borrow::Borrow<Q>, K: std::borrow::Borrow<Q>,
@@ -1759,6 +1765,8 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// # Errors /// # Errors
/// ///
/// `NonFiniteResult` if a sweep produces a NaN or infinite step. /// `NonFiniteResult` if a sweep produces a NaN or infinite step.
#[must_use = "this fit may have stopped at `max_iter` — check `converged`, \
or bind it to `_` to say you have decided not to"]
pub fn converge_partial(&mut self) -> Result<ConvergenceReport, InferenceError> { pub fn converge_partial(&mut self) -> Result<ConvergenceReport, InferenceError> {
use std::time::Instant; use std::time::Instant;
+3 -3
View File
@@ -104,12 +104,10 @@ use std::{
f64::consts::{FRAC_1_SQRT_2, FRAC_2_SQRT_PI, SQRT_2}, f64::consts::{FRAC_1_SQRT_2, FRAC_2_SQRT_PI, SQRT_2},
}; };
mod acquisition;
#[cfg(feature = "approx")] #[cfg(feature = "approx")]
mod approx; mod approx;
pub(crate) mod arena; pub(crate) mod arena;
mod time;
mod time_slice;
mod acquisition;
mod color_group; mod color_group;
mod competitor; mod competitor;
mod convergence; mod convergence;
@@ -130,6 +128,8 @@ mod predict;
pub(crate) mod quadrature; pub(crate) mod quadrature;
mod rating; mod rating;
pub(crate) mod storage; pub(crate) mod storage;
mod time;
mod time_slice;
pub use acquisition::expected_information_gain; pub use acquisition::expected_information_gain;
pub use convergence::{ConvergenceOptions, ConvergenceReport}; pub use convergence::{ConvergenceOptions, ConvergenceReport};
+1
View File
@@ -456,6 +456,7 @@ pub(crate) fn ranking_probability(
/// `Game::ranked` asks "what would we believe if *this* happened", which is /// `Game::ranked` asks "what would we believe if *this* happened", which is
/// what an expected-information-gain calculation needs alongside the weight. /// what an expected-information-gain calculation needs alongside the weight.
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, Debug, PartialEq)]
#[must_use]
pub struct Prediction { pub struct Prediction {
outcomes: Vec<(Vec<u32>, f64)>, outcomes: Vec<(Vec<u32>, f64)>,
} }
+1 -2
View File
@@ -11,7 +11,7 @@ use crate::{
/// ///
/// A configuration rather than a person: the per-history temporal state /// A configuration rather than a person: the per-history temporal state
/// (messages, last appearance) lives on `Competitor`. /// (messages, last appearance) lives on `Competitor`.
#[derive(Clone, Copy, Debug)] #[derive(Clone, Copy, Debug, PartialEq)]
pub struct Rating<T: Time = i64, D: Drift<T> = ConstantDrift> { pub struct Rating<T: Time = i64, D: Drift<T> = ConstantDrift> {
pub(crate) prior: Gaussian, pub(crate) prior: Gaussian,
pub(crate) beta: f64, pub(crate) beta: f64,
@@ -61,7 +61,6 @@ impl<T: Time, D: Drift<T>> Rating<T, D> {
} }
/// The configured prior skill estimate. /// The configured prior skill estimate.
#[must_use]
pub fn prior(&self) -> Gaussian { pub fn prior(&self) -> Gaussian {
self.prior self.prior
} }