refactor!: remove the inert online flag
Skill.online was initialised to N_INF and assigned nowhere, so HistoryBuilder::online(true) made every rating improper and log_evidence() reported n * ln(0.5) — every game scored as a coin flip. The value is finite and plausible, which is why it went unnoticed. The default was false, so no existing result changes. A working replacement lands next; a stored field cannot hold the quantity, because converge() alternates sweeps and contaminates skill.forward with backward information from the second iteration onward. Also renames a test binding from ..._online to ..._forward: it passes the forward flag, and the two senses being conflated is how this survived.
This commit is contained in:
+5
-22
@@ -29,7 +29,6 @@ pub struct HistoryBuilder<
|
|||||||
beta: f64,
|
beta: f64,
|
||||||
drift: D,
|
drift: D,
|
||||||
p_draw: f64,
|
p_draw: f64,
|
||||||
online: bool,
|
|
||||||
score_sigma: f64,
|
score_sigma: f64,
|
||||||
convergence: ConvergenceOptions,
|
convergence: ConvergenceOptions,
|
||||||
observer: O,
|
observer: O,
|
||||||
@@ -60,7 +59,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
|||||||
sigma: self.sigma,
|
sigma: self.sigma,
|
||||||
beta: self.beta,
|
beta: self.beta,
|
||||||
p_draw: self.p_draw,
|
p_draw: self.p_draw,
|
||||||
online: self.online,
|
|
||||||
score_sigma: self.score_sigma,
|
score_sigma: self.score_sigma,
|
||||||
convergence: self.convergence,
|
convergence: self.convergence,
|
||||||
observer: self.observer,
|
observer: self.observer,
|
||||||
@@ -87,11 +85,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
|||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn online(mut self, online: bool) -> Self {
|
|
||||||
self.online = online;
|
|
||||||
self
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Default observation noise for scored outcomes.
|
/// Default observation noise for scored outcomes.
|
||||||
///
|
///
|
||||||
/// # Panics
|
/// # Panics
|
||||||
@@ -135,7 +128,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
|||||||
beta: self.beta,
|
beta: self.beta,
|
||||||
drift: self.drift,
|
drift: self.drift,
|
||||||
p_draw: self.p_draw,
|
p_draw: self.p_draw,
|
||||||
online: self.online,
|
|
||||||
score_sigma: self.score_sigma,
|
score_sigma: self.score_sigma,
|
||||||
convergence: self.convergence,
|
convergence: self.convergence,
|
||||||
observer,
|
observer,
|
||||||
@@ -155,7 +147,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
|
|||||||
beta: self.beta,
|
beta: self.beta,
|
||||||
drift: self.drift,
|
drift: self.drift,
|
||||||
p_draw: self.p_draw,
|
p_draw: self.p_draw,
|
||||||
online: self.online,
|
|
||||||
score_sigma: self.score_sigma,
|
score_sigma: self.score_sigma,
|
||||||
convergence: self.convergence,
|
convergence: self.convergence,
|
||||||
observer: self.observer,
|
observer: self.observer,
|
||||||
@@ -171,7 +162,6 @@ impl Default for HistoryBuilder<i64, ConstantDrift, NullObserver, &'static str>
|
|||||||
beta: BETA,
|
beta: BETA,
|
||||||
drift: ConstantDrift(GAMMA),
|
drift: ConstantDrift(GAMMA),
|
||||||
p_draw: P_DRAW,
|
p_draw: P_DRAW,
|
||||||
online: false,
|
|
||||||
score_sigma: 1.0,
|
score_sigma: 1.0,
|
||||||
convergence: ConvergenceOptions::default(),
|
convergence: ConvergenceOptions::default(),
|
||||||
observer: NullObserver,
|
observer: NullObserver,
|
||||||
@@ -196,7 +186,6 @@ pub struct History<
|
|||||||
beta: f64,
|
beta: f64,
|
||||||
drift: D,
|
drift: D,
|
||||||
p_draw: f64,
|
p_draw: f64,
|
||||||
online: bool,
|
|
||||||
score_sigma: f64,
|
score_sigma: f64,
|
||||||
convergence: ConvergenceOptions,
|
convergence: ConvergenceOptions,
|
||||||
observer: O,
|
observer: O,
|
||||||
@@ -223,7 +212,6 @@ impl<K: Eq + Hash + Clone> History<i64, ConstantDrift, NullObserver, K> {
|
|||||||
beta: BETA,
|
beta: BETA,
|
||||||
drift: ConstantDrift(GAMMA),
|
drift: ConstantDrift(GAMMA),
|
||||||
p_draw: P_DRAW,
|
p_draw: P_DRAW,
|
||||||
online: false,
|
|
||||||
score_sigma: 1.0,
|
score_sigma: 1.0,
|
||||||
convergence: ConvergenceOptions::default(),
|
convergence: ConvergenceOptions::default(),
|
||||||
observer: NullObserver,
|
observer: NullObserver,
|
||||||
@@ -399,7 +387,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
|||||||
let per_slice: Vec<f64> = self
|
let per_slice: Vec<f64> = self
|
||||||
.time_slices
|
.time_slices
|
||||||
.par_iter()
|
.par_iter()
|
||||||
.map(|ts| ts.log_evidence(self.online, targets, forward, &self.agents))
|
.map(|ts| ts.log_evidence(targets, forward, &self.agents))
|
||||||
.collect();
|
.collect();
|
||||||
per_slice.into_iter().sum()
|
per_slice.into_iter().sum()
|
||||||
}
|
}
|
||||||
@@ -407,7 +395,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
|||||||
{
|
{
|
||||||
self.time_slices
|
self.time_slices
|
||||||
.iter()
|
.iter()
|
||||||
.map(|ts| ts.log_evidence(self.online, targets, forward, &self.agents))
|
.map(|ts| ts.log_evidence(targets, forward, &self.agents))
|
||||||
.sum()
|
.sum()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -935,12 +923,7 @@ mod tests {
|
|||||||
|
|
||||||
let w = [vec![1.0], vec![1.0]];
|
let w = [vec![1.0], vec![1.0]];
|
||||||
let p = Game::ranked_with_arena(
|
let p = Game::ranked_with_arena(
|
||||||
h.time_slices[1].events[0].within_priors(
|
h.time_slices[1].events[0].within_priors(false, &h.time_slices[1].skills, &h.agents),
|
||||||
false,
|
|
||||||
false,
|
|
||||||
&h.time_slices[1].skills,
|
|
||||||
&h.agents,
|
|
||||||
),
|
|
||||||
&[0.0, 1.0],
|
&[0.0, 1.0],
|
||||||
&w,
|
&w,
|
||||||
P_DRAW,
|
P_DRAW,
|
||||||
@@ -1180,11 +1163,11 @@ mod tests {
|
|||||||
let f = h.keys.get("f").unwrap();
|
let f = h.keys.get("f").unwrap();
|
||||||
|
|
||||||
let trueskill_log_evidence = h.log_evidence_internal(false, &[]);
|
let trueskill_log_evidence = h.log_evidence_internal(false, &[]);
|
||||||
let trueskill_log_evidence_online = h.log_evidence_internal(true, &[]);
|
let trueskill_log_evidence_forward = h.log_evidence_internal(true, &[]);
|
||||||
|
|
||||||
assert_ulps_eq!(
|
assert_ulps_eq!(
|
||||||
trueskill_log_evidence,
|
trueskill_log_evidence,
|
||||||
trueskill_log_evidence_online,
|
trueskill_log_evidence_forward,
|
||||||
epsilon = 1e-6
|
epsilon = 1e-6
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
+7
-14
@@ -22,7 +22,6 @@ pub(crate) struct Skill {
|
|||||||
backward: Gaussian,
|
backward: Gaussian,
|
||||||
likelihood: Gaussian,
|
likelihood: Gaussian,
|
||||||
pub(crate) elapsed: i64,
|
pub(crate) elapsed: i64,
|
||||||
pub(crate) online: Gaussian,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Skill {
|
impl Skill {
|
||||||
@@ -38,7 +37,6 @@ impl Default for Skill {
|
|||||||
backward: N_INF,
|
backward: N_INF,
|
||||||
likelihood: N_INF,
|
likelihood: N_INF,
|
||||||
elapsed: 0,
|
elapsed: 0,
|
||||||
online: N_INF,
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -59,7 +57,6 @@ struct Item {
|
|||||||
impl Item {
|
impl Item {
|
||||||
fn within_prior<T: Time, D: Drift<T>>(
|
fn within_prior<T: Time, D: Drift<T>>(
|
||||||
&self,
|
&self,
|
||||||
online: bool,
|
|
||||||
forward: bool,
|
forward: bool,
|
||||||
skills: &SkillStore,
|
skills: &SkillStore,
|
||||||
agents: &CompetitorStore<T, D>,
|
agents: &CompetitorStore<T, D>,
|
||||||
@@ -67,9 +64,7 @@ impl Item {
|
|||||||
let r = &agents[self.agent].rating;
|
let r = &agents[self.agent].rating;
|
||||||
let skill = skills.get(self.agent).unwrap();
|
let skill = skills.get(self.agent).unwrap();
|
||||||
|
|
||||||
if online {
|
if forward {
|
||||||
Rating::new(skill.online, r.beta, r.drift)
|
|
||||||
} else if forward {
|
|
||||||
Rating::new(skill.forward, r.beta, r.drift)
|
Rating::new(skill.forward, r.beta, r.drift)
|
||||||
} else {
|
} else {
|
||||||
Rating::new(skill.posterior() / self.likelihood, r.beta, r.drift)
|
Rating::new(skill.posterior() / self.likelihood, r.beta, r.drift)
|
||||||
@@ -107,7 +102,6 @@ impl Event {
|
|||||||
|
|
||||||
pub(crate) fn within_priors<T: Time, D: Drift<T>>(
|
pub(crate) fn within_priors<T: Time, D: Drift<T>>(
|
||||||
&self,
|
&self,
|
||||||
online: bool,
|
|
||||||
forward: bool,
|
forward: bool,
|
||||||
skills: &SkillStore,
|
skills: &SkillStore,
|
||||||
agents: &CompetitorStore<T, D>,
|
agents: &CompetitorStore<T, D>,
|
||||||
@@ -117,7 +111,7 @@ impl Event {
|
|||||||
.map(|team| {
|
.map(|team| {
|
||||||
team.items
|
team.items
|
||||||
.iter()
|
.iter()
|
||||||
.map(|item| item.within_prior(online, forward, skills, agents))
|
.map(|item| item.within_prior(forward, skills, agents))
|
||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
})
|
})
|
||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
@@ -136,7 +130,7 @@ impl Event {
|
|||||||
convergence: crate::ConvergenceOptions,
|
convergence: crate::ConvergenceOptions,
|
||||||
arena: &mut ScratchArena,
|
arena: &mut ScratchArena,
|
||||||
) -> EventUpdate {
|
) -> EventUpdate {
|
||||||
let teams = self.within_priors(false, false, skills, agents);
|
let teams = self.within_priors(false, skills, agents);
|
||||||
let result = self.outputs();
|
let result = self.outputs();
|
||||||
let g = match self.kind {
|
let g = match self.kind {
|
||||||
EventKind::Ranked => {
|
EventKind::Ranked => {
|
||||||
@@ -372,7 +366,7 @@ impl<T: Time> TimeSlice<T> {
|
|||||||
if from > 0 || self.color_groups.is_empty() {
|
if from > 0 || self.color_groups.is_empty() {
|
||||||
// Initial pass (add_events) or no color groups yet: simple sequential sweep.
|
// Initial pass (add_events) or no color groups yet: simple sequential sweep.
|
||||||
for event in self.events.iter_mut().skip(from) {
|
for event in self.events.iter_mut().skip(from) {
|
||||||
let teams = event.within_priors(false, false, &self.skills, agents);
|
let teams = event.within_priors(false, &self.skills, agents);
|
||||||
let result = event.outputs();
|
let result = event.outputs();
|
||||||
|
|
||||||
let g = match event.kind {
|
let g = match event.kind {
|
||||||
@@ -582,7 +576,6 @@ impl<T: Time> TimeSlice<T> {
|
|||||||
|
|
||||||
pub(crate) fn log_evidence<D: Drift<T>>(
|
pub(crate) fn log_evidence<D: Drift<T>>(
|
||||||
&self,
|
&self,
|
||||||
online: bool,
|
|
||||||
targets: &[Index],
|
targets: &[Index],
|
||||||
forward: bool,
|
forward: bool,
|
||||||
agents: &CompetitorStore<T, D>,
|
agents: &CompetitorStore<T, D>,
|
||||||
@@ -594,7 +587,7 @@ impl<T: Time> TimeSlice<T> {
|
|||||||
let mut arena = ScratchArena::new();
|
let mut arena = ScratchArena::new();
|
||||||
|
|
||||||
let run_event = |event: &Event, arena: &mut ScratchArena| -> f64 {
|
let run_event = |event: &Event, arena: &mut ScratchArena| -> f64 {
|
||||||
let teams = event.within_priors(online, forward, &self.skills, agents);
|
let teams = event.within_priors(forward, &self.skills, agents);
|
||||||
let result = event.outputs();
|
let result = event.outputs();
|
||||||
match event.kind {
|
match event.kind {
|
||||||
EventKind::Ranked => {
|
EventKind::Ranked => {
|
||||||
@@ -623,7 +616,7 @@ impl<T: Time> TimeSlice<T> {
|
|||||||
};
|
};
|
||||||
|
|
||||||
if targets.is_empty() {
|
if targets.is_empty() {
|
||||||
if online || forward {
|
if forward {
|
||||||
self.events
|
self.events
|
||||||
.iter()
|
.iter()
|
||||||
.map(|event| run_event(event, &mut arena))
|
.map(|event| run_event(event, &mut arena))
|
||||||
@@ -631,7 +624,7 @@ impl<T: Time> TimeSlice<T> {
|
|||||||
} else {
|
} else {
|
||||||
self.events.iter().map(|event| event.log_evidence).sum()
|
self.events.iter().map(|event| event.log_evidence).sum()
|
||||||
}
|
}
|
||||||
} else if online || forward {
|
} else if forward {
|
||||||
self.events
|
self.events
|
||||||
.iter()
|
.iter()
|
||||||
.filter(|event| {
|
.filter(|event| {
|
||||||
|
|||||||
Reference in New Issue
Block a user