Add `TeamRatingSystem` and `Rating` traits #10

jspspike · 2023-05-31T20:30:27Z

Each rating system has gone through the trouble having a similar or same interface so I thought it would be beneficial to add traits for the rating system and rating type. This way users could use the crate without having to specify the rating system and easily switch between different ones.

This PR only impls for trueskill at the moment

atomflunder · 2023-05-31T20:52:00Z

Hi, thanks for your PR!

This makes a lot of sense to me, one of the main features of this crate is supposed to be switching and comparing between rating systems easily and fast.

I think it looks quite good so far, and I would be happy to merge, but I think if we were to do it, we should probably implement it for every rating system and also every type of algorithm (1v1, Team v Team, etc.). This means to also include RatingSystem (1v1), RatingPeriodSystem and MultiTeamRatingSystem (names are obviously not set in stone) and of course implementing all of those for every algorithm currently available.

I would be more than happy to work on doing that, but I can only start working on it properly this weekend. So please feel free to add to this in whichever way you like in the meantime.

atomflunder · 2023-06-02T17:57:41Z

@jspspike Would you be okay with me adding the traits I described above to this existing PR?
While implementing those I did have some thoughts about the method names, for example the rating method should probably be called something like rate, in my opinion. Also I think it does make sense to add an uncertainty to the Rating trait, as most algorithms included have some form of this in their rating. For edge cases like volatility with Glicko-2 this would obviously not make sense to include those.

jspspike · 2023-06-02T20:04:23Z

@atomflunder Sure feel free to implement them, you could add directly to this branch. I think your proposals make sense, I guess the concern would be for edge cases where there isn't an uncertainty how to handle those cases. My opinion would be to omit uncertanity unless all implementations could use it but it's your decision on what you think the traits should be like

atomflunder · 2023-06-02T20:29:36Z

I guess the concern would be for edge cases where there isn't an uncertainty how to handle those cases. My opinion would be to omit uncertanity unless all implementations could use it but it's your decision on what you think the traits should be like

My thought process on this was to add a disclaimer in the documentation, and just return 0 for the cases where there is no uncertainty value.
I thought this would be better for the algorithms that rely heavily on the uncertainty value, for example TrueSkill where the original "display rank" is rating - 3* uncertainty.

To be honest I think both approaches are not 100% optimal, but I personally would prefer adding it. We can always change that later though, if it does end up confusing.

- RatingSystem - RatingPeriodSystem - MultiTeamRatingSystem

atomflunder · 2023-06-02T20:40:20Z

@jspspike I added the traits, let me know what you think and if you spot any mistakes. This should be fairly straightforward to add to the other algorithms from here on out, the hardest part is probably the design choices.
There is also still some documentation and new tests missing, but that won't be a big deal to add.

Edit: Oh and ignore that code coverage report obviously for now.

jspspike

One small change, otherwise it looks good to me

jspspike · 2023-06-04T17:51:22Z

src/lib.rs

+    fn rating(&self) -> f64;
+    /// A value for the uncertainty of a players rating.
+    /// If the algorithm does not include an uncertainty value, this will return `0.0` instead.
+    fn uncertainty(&self) -> f64;


I think an option makes more sense here

Yeah, I think you are right, this makes a lot more sense.

codecov-commenter · 2023-06-04T18:28:30Z

Codecov Report

Patch coverage has no change and project coverage change: -5.73 ⚠️

Comparison is base (398d621) 100.00% compared to head (73ecde9) 94.27%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@             Coverage Diff             @@
##            master      #10      +/-   ##
===========================================
- Coverage   100.00%   94.27%   -5.73%     
===========================================
  Files           13       13              
  Lines         7533     7987     +454     
===========================================
- Hits          7533     7530       -3     
- Misses           0      457     +457

Impacted Files	Coverage Δ
src/dwz.rs	`95.64% <0.00%> (-4.36%)`	⬇️
src/egf.rs	`88.09% <0.00%> (-11.91%)`	⬇️
src/elo.rs	`88.96% <0.00%> (-11.04%)`	⬇️
src/fifa.rs	`89.06% <0.00%> (-10.94%)`	⬇️
src/glicko.rs	`94.05% <0.00%> (-5.95%)`	⬇️
src/glicko2.rs	`95.89% <0.00%> (-4.11%)`	⬇️
src/glicko_boost.rs	`94.57% <0.00%> (-5.43%)`	⬇️
src/ingo.rs	`90.56% <0.00%> (-9.44%)`	⬇️
src/lib.rs	`100.00% <ø> (ø)`
src/sticko.rs	`94.61% <0.00%> (-5.39%)`	⬇️
... and 3 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

atomflunder · 2023-06-04T18:29:18Z

@jspspike So if you are okay with it, I would merge this, probably add some more tests and then make a new release for it.

Thank you again for your contribution!

jspspike · 2023-06-04T18:32:04Z

@atomflunder Looks good to me, thanks for taking the time and adding these traits

commit 65176ab Author: atomflunder <[email protected]> Date: Sun Jun 4 21:30:29 2023 +0200 Bump version to 0.25 commit 5432b61 Author: atomflunder <[email protected]> Date: Sun Jun 4 21:27:42 2023 +0200 Fix some new clippy lints commit be3a6df Author: atomflunder <[email protected]> Date: Sun Jun 4 21:21:07 2023 +0200 Add tests for traits commit 1276e87 Merge: 398d621 73ecde9 Author: atomflunder <[email protected]> Date: Sun Jun 4 20:33:50 2023 +0200 Merge pull request #10 from jspspike/master Add `TeamRatingSystem` and `Rating` traits commit 73ecde9 Author: atomflunder <[email protected]> Date: Sun Jun 4 20:26:53 2023 +0200 Use option return type for uncertainty method commit be3f64a Author: atomflunder <[email protected]> Date: Sat Jun 3 12:17:12 2023 +0200 Add traits for remaining algorithms commit 8da48e3 Author: atomflunder <[email protected]> Date: Fri Jun 2 22:35:42 2023 +0200 Add remaining traits for TrueSkill algorithm - RatingSystem - RatingPeriodSystem - MultiTeamRatingSystem commit 03bb12b Author: jspspike <[email protected]> Date: Wed May 31 13:40:21 2023 -0500 Add TeamRatingSystem trait commit 398d621 Author: atomflunder <[email protected]> Date: Fri May 19 18:40:41 2023 +0200 Update docs commit f6e0e38 Author: atomflunder <[email protected]> Date: Thu Mar 9 15:09:19 2023 +0100 Update TrueSkill documentation links

Was forgotten by me in the original PR

Add TeamRatingSystem trait

03bb12b

Add remaining traits for TrueSkill algorithm

8da48e3

- RatingSystem - RatingPeriodSystem - MultiTeamRatingSystem

Repository owner deleted a comment from codecov-commenter Jun 2, 2023

Add traits for remaining algorithms

be3f64a

Repository owner deleted a comment from codecov-commenter Jun 3, 2023

jspspike commented Jun 4, 2023

View reviewed changes

Use option return type for uncertainty method

73ecde9

atomflunder merged commit 1276e87 into atomflunder:master Jun 4, 2023

atomflunder mentioned this pull request Jun 6, 2024

Implement TrueSkill for more than two teams #11

Merged

atomflunder referenced this pull request Jun 8, 2024

Implement MultiTeamRatingSystem Trait for TrueSkill

4b82398

Was forgotten by me in the original PR

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `TeamRatingSystem` and `Rating` traits #10

Add `TeamRatingSystem` and `Rating` traits #10

jspspike commented May 31, 2023

atomflunder commented May 31, 2023 •

edited

Loading

atomflunder commented Jun 2, 2023

jspspike commented Jun 2, 2023

atomflunder commented Jun 2, 2023

atomflunder commented Jun 2, 2023 •

edited

Loading

jspspike left a comment

jspspike Jun 4, 2023

atomflunder Jun 4, 2023 •

edited

Loading

codecov-commenter commented Jun 4, 2023

atomflunder commented Jun 4, 2023 •

edited

Loading

jspspike commented Jun 4, 2023

Add TeamRatingSystem and Rating traits #10

Add TeamRatingSystem and Rating traits #10

Conversation

jspspike commented May 31, 2023

atomflunder commented May 31, 2023 • edited Loading

atomflunder commented Jun 2, 2023

jspspike commented Jun 2, 2023

atomflunder commented Jun 2, 2023

atomflunder commented Jun 2, 2023 • edited Loading

jspspike left a comment

Choose a reason for hiding this comment

jspspike Jun 4, 2023

Choose a reason for hiding this comment

atomflunder Jun 4, 2023 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Jun 4, 2023

Codecov Report

atomflunder commented Jun 4, 2023 • edited Loading

jspspike commented Jun 4, 2023

Add `TeamRatingSystem` and `Rating` traits #10

Add `TeamRatingSystem` and `Rating` traits #10

atomflunder commented May 31, 2023 •

edited

Loading

atomflunder commented Jun 2, 2023 •

edited

Loading

atomflunder Jun 4, 2023 •

edited

Loading

atomflunder commented Jun 4, 2023 •

edited

Loading