4
\$\begingroup\$

I have just started learning Rust and wanted some feedback on a simple program for calculating Elo ratings.

All of the following code was in a single file, but I imagine that this is the part that would be put into a library to be called by another program (see the next section).

enum Outcome {
    Win,
    Loss,
    Draw,
}

const WIN: f64 = 1.0;
const LOSS: f64 = 0.0;
const DRAW: f64 = 0.5;

// Returns the new ratings of two players of rating `r1` and `r2` given an outcome
fn elo(r1: i32, r2: i32, k: f64, outcome: Outcome) -> (i32, i32) {
    // Converting to float here to allow integer input in API
    let mut r1 = r1 as f64;
    let mut r2 = r2 as f64;

    // Probabilities of r1 and r2 winning
    let p1 = prob(r1, r2);
    let p2 = prob(r2, r1);

    match outcome {
        Outcome::Win => {
            r1 += k * (WIN - p1);
            r2 += k * (LOSS - p2);
        }
        Outcome::Loss => {
            r1 += k * (LOSS - p1);
            r2 += k * (WIN - p2);
        }
        Outcome::Draw => {
            r1 += k * (DRAW - p1);
            r2 += k * (DRAW - p2);
        }
    }

    (r1 as i32, r2 as i32)
}

// Returns the probability that a player with rating r1 beats a player with rating r2
fn prob(r1: f64, r2: f64) -> f64 {
    let diff = (r2 - r1) / 400.0;
    1.0 / (1.0 + f64::powf(10.0, diff))
}

And this is how I expect it would be used:

fn main() {
    // User sets their own k value
    const K: f64 = 30.0;

    // The ratings of the two players
    let r1 = 1600;
    let r2 = 1200;

    let (r1_new, r2_new) = elo(r1, r2, K, Outcome::Win);
    let r1_diff = r1_new - r1;
    let r2_diff = r2_new - r2;

    println!("Player 1 ({}) vs Player 2 ({})", r1, r2);
    println!(
        "Player 1 rating changed from {} to {} ({:+})",
        r1, r1_new, r1_diff
    );
    println!(
        "Player 2 rating changed from {} to {} ({:+})",
        r2, r2_new, r2_diff
    );
}

Any feedback on style, performance, layout etc. is appreciated, as I am a very new programmer.

\$\endgroup\$
1
  • 3
    \$\begingroup\$ What sticks out to me immediately is the variable naming: r1, r2, k and K etc. do not convey what they're supposed to represent. You at least tried to explain the first two ones in a comment, but that comment should be a docstring. \$\endgroup\$ Commented Aug 18, 2023 at 21:04

2 Answers 2

7
\$\begingroup\$
  • prob doesn't compute the probability of winning. If r1 == r2, the function returns 0.5 Do you think that equally rated players have a 50% probability of winning?

    What this function computes is, in full accordance with Elo system, an expected result. Call it so.

  • Once you compute the expected result r1 for a player 1, an expected result for a player 2 is 1 - r. There is no need to call the expensive powf for a second time.

  • It would be much simpler to pass numerical constants (WIN, LOSS, DRAW) as an actual result instead of Outcome. Or at least convert Outcome into an actual result in the beginning of elo, and recompute the ratings as

      match outcome {
          Outcome::Win  => { actual = WIN; }
          Outcome::Draw => { actual = DRAW; }
          Outcome::Loss => { actual = LOSS; }
      }
    
      expected = elo_expected_score(r1, r2);
      r1 += k * (actual - expected);
      r2 += k * (expected - actual);
    

    (Disclaimer: my Rust is a read-only. I am sure there is something more elegant and less error-prone than the matching above).

\$\endgroup\$
3
  • \$\begingroup\$ Nice suggestions, thanks! What's interesting is that having tested on 50,000,000 games a version that calls the powf function twice, versus using 1 - r, there was no performance difference. I'm not sure if this is a compiler optimization though. Either way, both versions finished in ~1 sec which is more than good enough for me! \$\endgroup\$
    – Blaire
    Commented Aug 19, 2023 at 9:59
  • 3
    \$\begingroup\$ The more idiomatic version would be let actual = match outcome { Outline::Win => WIN, /* etc. */ };. \$\endgroup\$
    – L. F.
    Commented Aug 19, 2023 at 11:36
  • \$\begingroup\$ "There is no need to call the expensive powf for a second time." +1. It makes it also clearer that p2 = 1 - p1 \$\endgroup\$
    – njzk2
    Commented Aug 19, 2023 at 14:31
3
\$\begingroup\$

A really nice and focused mini-program, that's great!

Naming

First things first, naming. Naming is hard, certainly, but it also matters.

For example, prob is rather... bland? Probability of what exactly? It would be clearer, instead, to name it probability_of_victory.

Similarly, elo is fairly bland too. compute_new_elo_ratings would be more descriptive.

Typing

Typeless values have no meaning. i32 is used for ELO rating here, but it could as easily be a distance, a duration, etc...

Even if focused on this one example, you use i32 as both:

  1. An ELO rating.
  2. The difference between two ELO ratings.

A fairly painless way to include types is to use tuple types in Rust:

struct KFactor(f64);
struct Rating(i32);
struct RatingDifference(i32);

And suddenly each value is clearly labelled, and mistakes are more easily caught at compile-time.

Conversion

The use of as is generally discouraged, for conversions. as will make the value fit, even if it means truncation, or other nonsense. It's lossy.

Instead, you should use the From/Into and TryFrom/TryInto conversion traits when possible:

  • From/Into are for infallible conversions, such as converting i32 to f64.
  • TryFrom/TryInto are for fallible conversions, though unfortunately it's not available for f64 to i32...

Formatting and Printing

Relatively recent versions of Rust allow directly mentioning an identifier in the format string, which is more convenient -- when the identifier exists.

That is: println!("Player 1 ({r1}) vs Player 2 ({r2})");

Eliminate redundancies

ELO is a very mathematical system, and therefore prob is symmetric, that is, p2 = 1.0 - p1. Similarly, LOSS = 1.0 - WIN, and DRAW = 1.0 - DRAW.

This means that when you compute:

r1 += k * (WIN - p1);
r2 += k * (LOSS - p2);

If you put the substitutions in place, you get:

LOSS - p2 = (1.0 - WIN) - (1.0 - p1) = - (WIN - p1)

That is, the number of ranking points earned by player 1 are the number of ranking points lost by player 2, and vice versa.

You are, therefore doing twice as many calculations as necessary. Not a problem of itself, except for the code duplication, and the risk of making a typo...

Factorize related code

Unlike C, in Rust enums can have associated functions. Rather than exposing the WIN, LOSS, and DRAW constants, and having the user match on Outcome and map each outcome to the appropriate constant -- which they may do wrong -- it would be better for Outcome to have a function that gives the matching value.

Tests!

It would be good to add some tests, to verify that basic properties hold. For example, that the probability of winning of two equally ranked players is 0.5.

In Rust, it's idiomatic to wrap tests in a test-only module with:

#[cfg(test)]
mod tests {
} // mod tests

Then, a test is just a function annotated with #[test].

Putting it altogether

The Rust Playground is an online code editor specialized for Rust. While by default it can run a small program, or some tests, it also features a variety of tools such as rustfmt (code formatting) or clippy (linting) and a variety of outputs (MIR, LLVM IR, assembly, etc...).

In the context of Stack Exchange, it also makes it very easy to share code! For example, here is the rewritten code integrating the advice above: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=df6b618cffdd369ce95156edcd4c092b

fn main() {
    const K: KFactor = KFactor(30.0);

    let r1 = Ranking(1600);
    let r2 = Ranking(1200);

    let difference = compute_elo_ranking_evolution(r1, r2, K, Outcome::Win);

    println!("Player 1 ({}) vs Player 2 ({})", r1.0, r2.0);
    println!(
        "Player 1 rating changed from {} to {} ({:+})",
        r1.0,
        r1.0 + difference.0,
        difference.0
    );
    println!(
        "Player 2 rating changed from {} to {} ({:+})",
        r2.0,
        r2.0 - difference.0,
        -difference.0
    );
}

//
//  Implementation
//

#[derive(Clone, Copy, Debug)]
struct KFactor(f64);

#[derive(Clone, Copy, Debug)]
struct Ranking(i32);

#[derive(Clone, Copy, Debug)]
struct RankingDifference(i32);

//  The outcome of a match between two players, for the first player.
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
enum Outcome {
    Win,
    Draw,
    Loss,
}

impl Outcome {
    fn no_idea_what_to_name_it(self) -> f64 {
        match self {
            Self::Win => 1.0,
            Self::Draw => 0.5,
            Self::Loss => 0.0,
        }
    }
}

//  Returns the difference in rank following the outcome of the match.
//
//  The difference must be added to player 1's ranking, and deduced from player 2's ranking.
fn compute_elo_ranking_evolution(
    r1: Ranking,
    r2: Ranking,
    k: KFactor,
    outcome: Outcome,
) -> RankingDifference {
    let difference = k.0 * (outcome.no_idea_what_to_name_it() - probability_of_victory(r1, r2));

    RankingDifference(difference.round() as _)
}

//  Returns the probability that a player of ranking `r1` would win over a
//  player of ranking `r2`.
//
//  The return value is always in the `0...=1.` range.
fn probability_of_victory(r1: Ranking, r2: Ranking) -> f64 {
    let difference: f64 = (r2.0 - r1.0).into();

    1.0 / (1.0 + f64::powf(10.0, difference / 400.0))
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn probability_of_victory_equal_rankings() {
        for i in 1..100 {
            let ranking = Ranking(i * 100);

            assert_eq!(
                0.5,
                probability_of_victory(ranking, ranking),
                "for {ranking:?}"
            );
        }
    }

    #[test]
    fn probability_of_victory_symmetry() {
        const R1: Ranking = Ranking(1600);
        const R2: Ranking = Ranking(1200);

        let r1_win = probability_of_victory(R1, R2);
        let r2_win = probability_of_victory(R2, R1);

        let expected_r2_win = 1. - r1_win;

        let divergence = (expected_r2_win - r2_win).abs() / r2_win.abs();

        assert!(
            divergence < 1e-6,
            "{r1_win}, {r2_win}, {expected_r2_win}, {divergence}"
        );
    }

    #[test]
    fn ranking_evolution() {
        const K: KFactor = KFactor(30.0);
        const R1: Ranking = Ranking(1600);
        const R2: Ranking = Ranking(1200);

        let win = compute_elo_ranking_evolution(R1, R2, K, Outcome::Win);
        let draw = compute_elo_ranking_evolution(R1, R2, K, Outcome::Draw);
        let loss = compute_elo_ranking_evolution(R1, R2, K, Outcome::Loss);

        assert_eq!(3, win.0);
        assert_eq!(-12, draw.0);
        assert_eq!(-27, loss.0);
    }
} // mod tests

Note: Yes, a Rust binary can also contain test, though the playground no longer allows running the binary in such a case :P

\$\endgroup\$
1
  • \$\begingroup\$ "The use of as is generally discouraged" - didn't know this and have been using it frequently. Thanks for the feedback! \$\endgroup\$
    – Blaire
    Commented Aug 20, 2023 at 10:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.