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

refactor!: make Competitor::message an Option, and compute_elapsed loud

Browse Source 
Two of the five remaining items on #23.

`Competitor.message` was a `Gaussian` using the improper `N_INF` as an "unset"
sentinel, so `message != N_INF` meant "has a message" and every reader had to
know that convention. It is now `Option<Gaussian>`, which makes "no message
yet" and "a legitimately improper message" distinguishable at the type level
instead of by float comparison.

Worth noting what the change surfaced: switching the type turned every read
site into a compile error, and there were eight — two in the convergence sweep,
five in ingestion, one in new_backward_info. The last is the interesting one:
`skill.backward = agents[agent].message` needed `unwrap_or(N_INF)` rather than
an unwrap, because an absent message genuinely does mean the improper identity
there. A sentinel-based refactor would have had to find that by reading.

This is a breaking change: `message` is a public field. It rides the next
minor bump.

`compute_elapsed` clamped a negative elapsed to zero silently. Negative elapsed
means slices are being visited out of time order, which would otherwise make
drift *reduce* uncertainty. Release still clamps, so a bad timestamp degrades
to "no drift" rather than corrupting a posterior, but debug now trips — getting
there is a slice-ordering bug, not something callers can cause with ordinary
data.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
This commit is contained in:
logaritmisk
2026-08-27 17:46:12 +02:00
co-authored by Claude Opus 5
parent 9b2c2b38c8
commit 06ed24b240
3 changed files with 51 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/competitor.rs
+19 -13
View File
@@ -1,5 +1,4 @@
use crate::{
N_INF,
drift::{ConstantDrift, Drift},
gaussian::Gaussian,
rating::Rating,
@@ -13,7 +12,14 @@ use crate::{
#[derive(Debug)]
pub struct Competitor<T: Time = i64, D: Drift<T> = ConstantDrift> {
pub rating: Rating<T, D>,
pub message: Gaussian,
/// The forward message carried from this competitor's last appearance, or
/// `None` before they have appeared anywhere.
///
/// Previously an improper `N_INF` served as the unset sentinel, which made
/// "no message yet" indistinguishable from "a legitimately improper
/// message" at the type level and required every reader to know the
/// convention.
pub message: Option<Gaussian>,
pub last_time: Option<T>,
}
@@ -21,14 +27,16 @@ impl<T: Time, D: Drift<T>> Competitor<T, D> {
/// Compute the message received at time `now`, with drift accumulated
/// from `self.last_time` (if any) to `now`.
pub(crate) fn receive(&self, now: &T) -> Gaussian {
if self.message != N_INF {
match self.message {
Some(message) => {
let elapsed_variance = match &self.last_time {
Some(last) => self.rating.drift.variance_delta(last, now),
None => 0.0,
};
self.message.forget(elapsed_variance)
} else {
self.rating.prior
message.forget(elapsed_variance)
}
None => self.rating.prior,
}
}
@@ -37,11 +45,9 @@ impl<T: Time, D: Drift<T>> Competitor<T, D> {
/// Used in convergence sweeps where the elapsed was cached at slice-construction time
/// and should not be recomputed from `last_time` (which may have shifted).
pub(crate) fn receive_for_elapsed(&self, elapsed: i64) -> Gaussian {
if self.message != N_INF {
self.message
.forget(self.rating.drift.variance_for_elapsed(elapsed))
} else {
self.rating.prior
match self.message {
Some(message) => message.forget(self.rating.drift.variance_for_elapsed(elapsed)),
None => self.rating.prior,
}
}
}
@@ -50,7 +56,7 @@ impl Default for Competitor<i64, ConstantDrift> {
fn default() -> Self {
Self {
rating: Rating::default(),
message: N_INF,
message: None,
last_time: None,
}
}
@@ -63,7 +69,7 @@ where
C: Iterator<Item = &'a mut Competitor<T, D>>,
{
for c in competitors {
c.message = N_INF;
c.message = None;
if last_time {
c.last_time = None;
}
src/history.rs
+8 -8
View File
@@ -1,7 +1,7 @@
use std::{borrow::Borrow, collections::HashMap, hash::Hash, marker::PhantomData};
use crate::{
BETA, GAMMA, Index, MU, N_INF, P_DRAW, SIGMA,
BETA, GAMMA, Index, MU, P_DRAW, SIGMA,
competitor::{self, Competitor},
convergence::{ConvergenceOptions, ConvergenceReport},
drift::{ConstantDrift, Drift},
@@ -254,7 +254,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
for j in (0..self.time_slices.len() - 1).rev() {
for agent in self.time_slices[j + 1].skills.keys() {
self.agents.get_mut(agent).unwrap().message =
self.time_slices[j + 1].backward_prior_out(&agent, &self.agents);
Some(self.time_slices[j + 1].backward_prior_out(&agent, &self.agents));
}
let old = self.time_slices[j].posteriors();
@@ -273,7 +273,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
for j in 1..self.time_slices.len() {
for agent in self.time_slices[j - 1].skills.keys() {
self.agents.get_mut(agent).unwrap().message =
self.time_slices[j - 1].forward_prior_out(&agent);
Some(self.time_slices[j - 1].forward_prior_out(&agent));
}
let old = self.time_slices[j].posteriors();
@@ -723,7 +723,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
self.drift,
)
}),
message: N_INF,
message: None,
last_time: None,
},
);
@@ -761,7 +761,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
let agent = self.agents.get_mut(*agent_idx).unwrap();
agent.last_time = Some(time_slice.time);
agent.message = time_slice.forward_prior_out(agent_idx);
agent.message = Some(time_slice.forward_prior_out(agent_idx));
}
}
@@ -794,7 +794,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
let agent = self.agents.get_mut(agent_idx).unwrap();
agent.last_time = Some(t);
agent.message = time_slice.forward_prior_out(&agent_idx);
agent.message = Some(time_slice.forward_prior_out(&agent_idx));
}
k += 1;
@@ -810,7 +810,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
let agent = self.agents.get_mut(agent_idx).unwrap();
agent.last_time = Some(t);
agent.message = time_slice.forward_prior_out(&agent_idx);
agent.message = Some(time_slice.forward_prior_out(&agent_idx));
}
k += 1;
@@ -834,7 +834,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
let agent = self.agents.get_mut(*agent_idx).unwrap();
agent.last_time = Some(time_slice.time);
agent.message = time_slice.forward_prior_out(agent_idx);
agent.message = Some(time_slice.forward_prior_out(agent_idx));
}
}
src/time_slice.rs
+20 -2
View File
@@ -581,7 +581,7 @@ impl<T: Time> TimeSlice<T> {
pub(crate) fn new_backward_info<D: Drift<T>>(&mut self, agents: &CompetitorStore<T, D>) {
for (agent, skill) in self.skills.iter_mut() {
skill.backward = agents[agent].message;
skill.backward = agents[agent].message.unwrap_or(N_INF);
}
self.iteration(0, agents);
}
@@ -761,8 +761,26 @@ impl<T: Time> TimeSlice<T> {
}
}
/// Elapsed time from a competitor's previous appearance to `current`.
///
/// A negative elapsed means slices are being visited out of time order, which
/// would make drift *reduce* uncertainty. Release builds clamp to zero so a
/// bad timestamp degrades to "no drift" rather than corrupting the posterior;
/// debug builds trip instead, because reaching here is a bug in slice ordering
/// rather than something callers can cause with ordinary data.
pub(crate) fn compute_elapsed<T: Time>(last: Option<&T>, current: &T) -> i64 {
last.map(|l| l.elapsed_to(current).max(0)).unwrap_or(0)
let Some(last) = last else {
return 0;
};
let elapsed = last.elapsed_to(current);
debug_assert!(
elapsed >= 0,
"negative elapsed ({elapsed}) — slices visited out of time order"
);
elapsed.max(0)
}
#[cfg(test)]