fix: warn on dropped builders and values; stop exporting EP internals
`h.event(1).team(["x"]).team(["y"]).ranking([0, 1]);` without the terminal `.commit()` was a silent no-op: no warning, no error, and the next thing the caller does is converge an empty history and read `None` skills. `EventBuilder` already carried a `#[must_use]`; the value types around it did not, so the same silence covered `Team::with_members`, `Member::new`, `Outcome::*`, `Joint` and `Prediction::outcomes`. `#[must_use]` now goes on the *types* rather than being sprinkled over methods, which covers every constructor and builder setter at once and gives the crate a rule where it previously had a list. Verified by compiling a program that drops each one and reading the warnings back, rather than by assuming the attribute took. Visibility, from #73: `Gaussian::damp_natural` was reachable from outside the crate despite being an EP damping internal called only from `src/factor/`. The stray `pub fn`s inside the private `time_slice`, `key_table` and `matrix` modules are now `pub(crate)`, so their visibility states what it means instead of relying on the module being private. `storage/mod.rs` and `factor/mod.rs` become `storage.rs` and `factor.rs`. Closes #67. Refs #73. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
+2
-1
@@ -20,12 +20,12 @@ pub struct Event<T: Time, K> {
|
||||
|
||||
/// A team: list of members competing together.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
#[must_use]
|
||||
pub struct Team<K> {
|
||||
pub members: SmallVec<[Member<K>; 4]>,
|
||||
}
|
||||
|
||||
impl<K> Team<K> {
|
||||
#[must_use]
|
||||
pub fn new() -> Self {
|
||||
Self {
|
||||
members: SmallVec::new(),
|
||||
@@ -62,6 +62,7 @@ impl<K> Default for Team<K> {
|
||||
/// `InferenceError::ConflictingCompetitorConfig`: events in a batch have no
|
||||
/// order, so there would be no well-defined winner.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
#[must_use]
|
||||
pub struct Member<K> {
|
||||
pub key: K,
|
||||
pub weight: f64,
|
||||
|
||||
+1
-1
@@ -243,7 +243,7 @@ impl Gaussian {
|
||||
/// Used by within-game inference to stabilise oscillating fixed-point
|
||||
/// loops on hard graphs. `alpha = 1.0` returns `new` exactly;
|
||||
/// `alpha < 1.0` shrinks each per-step update.
|
||||
pub fn damp_natural(self, new: Gaussian, alpha: f64) -> Gaussian {
|
||||
pub(crate) fn damp_natural(self, new: Gaussian, alpha: f64) -> Gaussian {
|
||||
Gaussian::from_natural(
|
||||
alpha * new.pi() + (1.0 - alpha) * self.pi(),
|
||||
alpha * new.tau() + (1.0 - alpha) * self.tau(),
|
||||
|
||||
@@ -2630,6 +2630,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> std::fmt::Debug
|
||||
/// Slices a competitor sits out cost nothing: an absence is not an appearance,
|
||||
/// so a competitor seen in the first and last of a hundred slices contributes
|
||||
/// two variables, not a hundred.
|
||||
#[must_use]
|
||||
pub struct Joint<'h, T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> {
|
||||
history: &'h History<T, D, O, K>,
|
||||
cholesky: crate::joint::Cholesky,
|
||||
|
||||
+9
-6
@@ -26,21 +26,24 @@ where
|
||||
K: Eq + Hash + Clone,
|
||||
{
|
||||
#[must_use]
|
||||
pub fn new() -> Self {
|
||||
pub(crate) fn new() -> Self {
|
||||
Self {
|
||||
forward: HashMap::new(),
|
||||
reverse: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get<Q: ?Sized + Hash + Eq>(&self, k: &Q) -> Option<Index>
|
||||
pub(crate) fn get<Q: ?Sized + Hash + Eq>(&self, k: &Q) -> Option<Index>
|
||||
where
|
||||
K: Borrow<Q>,
|
||||
{
|
||||
self.forward.get(k).cloned()
|
||||
}
|
||||
|
||||
pub fn get_or_create<Q: ?Sized + Hash + Eq + ToOwned<Owned = K>>(&mut self, k: &Q) -> Index
|
||||
pub(crate) fn get_or_create<Q: ?Sized + Hash + Eq + ToOwned<Owned = K>>(
|
||||
&mut self,
|
||||
k: &Q,
|
||||
) -> Index
|
||||
where
|
||||
K: Borrow<Q>,
|
||||
{
|
||||
@@ -56,7 +59,7 @@ where
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn key(&self, idx: Index) -> Option<&K> {
|
||||
pub(crate) fn key(&self, idx: Index) -> Option<&K> {
|
||||
self.reverse.get(idx.0)
|
||||
}
|
||||
|
||||
@@ -66,12 +69,12 @@ where
|
||||
/// 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> {
|
||||
pub(crate) fn keys(&self) -> impl ExactSizeIterator<Item = &K> {
|
||||
self.reverse.iter()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn len(&self) -> usize {
|
||||
pub(crate) fn len(&self) -> usize {
|
||||
self.reverse.len()
|
||||
}
|
||||
}
|
||||
|
||||
+5
-5
@@ -140,7 +140,7 @@ impl Lu {
|
||||
}
|
||||
|
||||
impl Matrix {
|
||||
pub fn new(height: usize, width: usize) -> Matrix {
|
||||
pub(crate) fn new(height: usize, width: usize) -> Matrix {
|
||||
Matrix {
|
||||
data: vec![0.0; height * width].into_boxed_slice(),
|
||||
height,
|
||||
@@ -148,7 +148,7 @@ impl Matrix {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn transpose(&self) -> Matrix {
|
||||
pub(crate) fn transpose(&self) -> Matrix {
|
||||
let mut matrix = Matrix::new(self.width, self.height);
|
||||
|
||||
for c in 0..self.width {
|
||||
@@ -166,7 +166,7 @@ impl Matrix {
|
||||
/// # Panics
|
||||
///
|
||||
/// Panics if the matrix is not square.
|
||||
pub fn determinant(&self) -> f64 {
|
||||
pub(crate) fn determinant(&self) -> f64 {
|
||||
assert_eq!(
|
||||
self.width, self.height,
|
||||
"determinant requires a square matrix, got {}x{}",
|
||||
@@ -184,7 +184,7 @@ impl Matrix {
|
||||
///
|
||||
/// See [`Lu::ln_abs_determinant`] for why a ratio of determinants must be
|
||||
/// taken this way.
|
||||
pub fn ln_abs_determinant(&self) -> f64 {
|
||||
pub(crate) fn ln_abs_determinant(&self) -> f64 {
|
||||
assert_eq!(
|
||||
self.width, self.height,
|
||||
"determinant requires a square matrix, got {}x{}",
|
||||
@@ -203,7 +203,7 @@ impl Matrix {
|
||||
/// # Panics
|
||||
///
|
||||
/// Panics if the matrix is not square or is singular.
|
||||
pub fn inverse(&self) -> Matrix {
|
||||
pub(crate) fn inverse(&self) -> Matrix {
|
||||
assert_eq!(
|
||||
self.width, self.height,
|
||||
"inverse requires a square matrix, got {}x{}",
|
||||
|
||||
+1
-2
@@ -16,6 +16,7 @@ use smallvec::SmallVec;
|
||||
/// when `Some`; `None` inherits the history default.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
#[non_exhaustive]
|
||||
#[must_use]
|
||||
pub enum Outcome {
|
||||
Ranked(SmallVec<[u32; 4]>),
|
||||
#[non_exhaustive]
|
||||
@@ -46,7 +47,6 @@ impl Outcome {
|
||||
/// `p_draw > 0`. Asking "team 5 won" and silently getting "everyone drew"
|
||||
/// is exactly the class of quiet wrong answer this crate keeps removing, so
|
||||
/// the check happens here where the mistake is.
|
||||
#[must_use]
|
||||
pub fn winner(winner: u32, n: u32) -> Self {
|
||||
Self::try_winner(winner, n)
|
||||
.unwrap_or_else(|_| panic!("winner index {winner} out of range 0..{n}"))
|
||||
@@ -73,7 +73,6 @@ impl Outcome {
|
||||
}
|
||||
|
||||
/// All `n` teams tied.
|
||||
#[must_use]
|
||||
pub fn draw(n: u32) -> Self {
|
||||
Self::Ranked(SmallVec::from_vec(vec![0; n as usize]))
|
||||
}
|
||||
|
||||
@@ -467,6 +467,7 @@ impl Prediction {
|
||||
}
|
||||
|
||||
/// Every possible finishing order and its probability, most likely first.
|
||||
#[must_use]
|
||||
pub fn outcomes(&self) -> impl ExactSizeIterator<Item = (&[u32], f64)> {
|
||||
self.outcomes.iter().map(|(r, p)| (r.as_slice(), *p))
|
||||
}
|
||||
|
||||
+9
-5
@@ -228,7 +228,7 @@ pub struct TimeSlice<T: Time = i64> {
|
||||
}
|
||||
|
||||
impl<T: Time> TimeSlice<T> {
|
||||
pub fn new(time: T, p_draw: f64, convergence: crate::ConvergenceOptions) -> Self {
|
||||
pub(crate) fn new(time: T, p_draw: f64, convergence: crate::ConvergenceOptions) -> Self {
|
||||
Self {
|
||||
events: Vec::new(),
|
||||
skills: SkillStore::new(),
|
||||
@@ -282,7 +282,7 @@ impl<T: Time> TimeSlice<T> {
|
||||
);
|
||||
}
|
||||
|
||||
pub fn add_events<D: Drift<T>>(
|
||||
pub(crate) fn add_events<D: Drift<T>>(
|
||||
&mut self,
|
||||
composition: Vec<Vec<Vec<Index>>>,
|
||||
results: Option<Vec<Vec<f64>>>,
|
||||
@@ -393,7 +393,11 @@ impl<T: Time> TimeSlice<T> {
|
||||
/// Panics if an event references a competitor with no entry in this
|
||||
/// slice's skill store. `add_events` inserts one for every participant, so
|
||||
/// this cannot happen for slices built through the public API.
|
||||
pub fn iteration<D: Drift<T>>(&mut self, from: usize, competitors: &CompetitorStore<T, D>) {
|
||||
pub(crate) fn iteration<D: Drift<T>>(
|
||||
&mut self,
|
||||
from: usize,
|
||||
competitors: &CompetitorStore<T, D>,
|
||||
) {
|
||||
if from == 0 && self.color_groups_dirty {
|
||||
self.recompute_color_groups();
|
||||
}
|
||||
@@ -782,7 +786,7 @@ 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(crate) fn get_composition(&self) -> Vec<Vec<Vec<Index>>> {
|
||||
self.events
|
||||
.iter()
|
||||
.map(|event| {
|
||||
@@ -802,7 +806,7 @@ impl<T: Time> TimeSlice<T> {
|
||||
|
||||
/// Test-only: reads the slice's shape back for assertions.
|
||||
#[cfg(test)]
|
||||
pub fn get_results(&self) -> Vec<Vec<f64>> {
|
||||
pub(crate) fn get_results(&self) -> Vec<Vec<f64>> {
|
||||
self.events
|
||||
.iter()
|
||||
.map(|event| {
|
||||
|
||||
Reference in New Issue
Block a user