refactor!: remove ConvergenceReport::slices_skipped
Closes #33. The field was public, hardcoded to 0 at both construction sites, and had no route to ever being non-zero. It was added in T3 as the reporting surface for dirty-bit slice skipping. That feature is #4, closed as unworkable — the ceiling measured at ~6% against a projected 5-50x, on top of three independent soundness blockers. #32, which reattributed the cost to ingestion, is closed too: the re-convergence is necessary work rather than waste, because appending one event genuinely moves the involved competitors ~1.2 sigma across their whole history. Nothing left would ever populate it. This is the same defect class as #19, where this arc started: a public surface that looks implemented, reports a plausible value, and is inert. A caller reading `slices_skipped: 0` reasonably concludes "no slices were skipped this run", not "this feature does not exist". Removed rather than documented as reserved. Its only value was as a hook for a plan that no longer exists, and keeping it preserves the shape of that plan. Breaking, but ConvergenceReport is returned rather than constructed by callers, so the only breakage is code reading a constant zero. Also added a test asserting every remaining field carries real information — iterations non-zero, final_step finite, log_evidence a finite negative log probability, and per_iteration_time holding one duration per iteration. Mutation-proved: pinning per_iteration_time to an empty SmallVec fails it. The next always-constant member now has to survive an assertion rather than just a reviewer's attention, which is the actual lesson of #19 and #33. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
This commit is contained in:
@@ -38,7 +38,6 @@ pub struct ConvergenceReport {
|
|||||||
pub log_evidence: f64,
|
pub log_evidence: f64,
|
||||||
pub converged: bool,
|
pub converged: bool,
|
||||||
pub per_iteration_time: SmallVec<[Duration; 32]>,
|
pub per_iteration_time: SmallVec<[Duration; 32]>,
|
||||||
pub slices_skipped: usize,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
|||||||
@@ -603,7 +603,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
|||||||
log_evidence: 0.0,
|
log_evidence: 0.0,
|
||||||
converged: true,
|
converged: true,
|
||||||
per_iteration_time: SmallVec::new(),
|
per_iteration_time: SmallVec::new(),
|
||||||
slices_skipped: 0,
|
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -642,7 +641,6 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
|||||||
log_evidence,
|
log_evidence,
|
||||||
converged,
|
converged,
|
||||||
per_iteration_time: per_iter,
|
per_iteration_time: per_iter,
|
||||||
slices_skipped: 0,
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -247,3 +247,46 @@ fn fluent_event_builder_scores() {
|
|||||||
let b = h.current_skill(&"bob").unwrap();
|
let b = h.current_skill(&"bob").unwrap();
|
||||||
assert!(a.mu() > b.mu());
|
assert!(a.mu() > b.mu());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Every field of `ConvergenceReport` must carry real information.
|
||||||
|
///
|
||||||
|
/// `slices_skipped` was public, hardcoded to `0`, and reported a plausible
|
||||||
|
/// value for a feature that never existed — the same shape as the inert
|
||||||
|
/// `online` flag in #19. It was removed in #33. This pins the remaining fields
|
||||||
|
/// so the next always-constant member has to survive an assertion rather than
|
||||||
|
/// just a reviewer's attention.
|
||||||
|
#[test]
|
||||||
|
fn every_convergence_report_field_is_populated() {
|
||||||
|
let mut h = History::builder().build();
|
||||||
|
|
||||||
|
for time in 1..=6i64 {
|
||||||
|
h.record_winner(&"a", &"b", time).unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
let report = h.converge().unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
report.iterations > 0,
|
||||||
|
"iterations is zero on a real converge"
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(report.converged, "fixture must converge");
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
report.final_step.0.is_finite() && report.final_step.1.is_finite(),
|
||||||
|
"final_step is not finite: {:?}",
|
||||||
|
report.final_step
|
||||||
|
);
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
report.log_evidence.is_finite() && report.log_evidence < 0.0,
|
||||||
|
"log_evidence is not a finite negative log probability: {}",
|
||||||
|
report.log_evidence
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
report.per_iteration_time.len(),
|
||||||
|
report.iterations,
|
||||||
|
"per_iteration_time must carry one duration per iteration"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user