Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add partial play weights to the full TrueSkill calculations #12

Open
atomflunder opened this issue Jun 8, 2024 · 5 comments
Open

Add partial play weights to the full TrueSkill calculations #12

atomflunder opened this issue Jun 8, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@atomflunder
Copy link
Owner

atomflunder commented Jun 8, 2024

TrueSkill has the capability for partial play weights, accounting for players who have not played the whole match. These can range from 0.0 (no participation) to 1.0 (full match).

This crate should probably implement these, although I am undecided on how to best do it to be as user-friendly as possible. Passing a partial play weight in every time could be pretty annoying to the user, since in 99.9% of cases, it will be set to 1.0.

Rust doesn't use default values for arguments unfortunately, I think those would come in useful here.

// How it is currently done:
pub fn trueskill_multi_team(
    teams_and_ranks: &[(&[TrueSkillRating], MultiTeamOutcome)],
    config: &TrueSkillConfig,
) -> Vec<Vec<TrueSkillRating>>

// This seems annoying?
// Would also interfere with the RatingSystem Trait
pub fn trueskill_multi_team(
    teams_and_ranks: &[(&[(TrueSkillRating, f64)], MultiTeamOutcome)],
    config: &TrueSkillConfig,
) -> Vec<Vec<TrueSkillRating>>

Maybe it's useful to offer 2 different functions, one with and one without weights? Does seem a bit "bloaty" to me though.

Also, I think this only makes sense to implement for the full trueskill_multi_team and match_quality_multi_team functions, and not the 1-vs-1 and Team-vs-Team shortcuts.
For the match quality, the Matrix implementation also has to be updated in two places in the create_rotated_a_matrix function.
Not sure about the expected_score_multi_team function at the moment.

I think the technical implementation will be fairly straight-forward, but the design choices will be more difficult here. I also wouldn't call this issue urgent, but it would be nice to have eventually, before an eventual 1.0 release.

@atomflunder atomflunder added the enhancement New feature or request label Jun 8, 2024
@atomflunder atomflunder self-assigned this Jun 8, 2024
@asyncth
Copy link
Contributor

asyncth commented Oct 11, 2024

I want to give this a go, tomorrow perhaps? Or the day after tomorrow.

@atomflunder
Copy link
Owner Author

I want to give this a go, tomorrow perhaps? Or the day after

Feel free to do that, as I said I don't think it will be that difficult to implement from a technical standpoint.
But how to pass the weights into the function in a clean way is something that isn't straightforward. The function parameters are already a lot (Maybe type aliases for teams would help?). I've been thinking about it yesterday and I am not sure what's the most user-friendly approach. Other libraries just have the weights as a separate argument, like this:

// ts-trueskill library
export function rate(
  ratingGroups: Rating[][] | any[],
  ranks?: any[] | null,
  weights?: any[] | null,
  minDelta = 0.0001,
  env: TrueSkill = new TrueSkill(),
): Rating[][] {
  // ...
}

Which I do think is a bit cleaner than the proposed stuff from me in the original comment, but then you have to validate those separately.
It also has the advantage of not breaking the MultiTeamRatingSystem Trait, since the Weng-Lin algorithm does not support weights. I know that the OpenSkill library supports weights, but it's more of a bonus and not something supported in the original proposal of the algorithm.

But it seems like a hassle for the user to do add the weights regardless, especially since most users would probably leave all of the weights at 1.0 at all times. Maybe a helper function would be neat?

fn get_default_weights(teams: &[&[TrueSkillRating]]) -> Vec<Vec<f64>> {
  // Just returns a Vec of Vec of 1.0 for every player
}

Generally speaking, I am quite unhappy about the API choices in this library that I made in the past. On my other libraries that no one uses I would not mind dropping massive breaking changes, but this is the first project on mine that is kind of in use, so I'm very careful with that.

Sorry if this is a long ramble, if you have an idea of how to go about this please let me know @asyncth .
By the way, I really appreciate your contributions to the library 🙂

@asyncth
Copy link
Contributor

asyncth commented Oct 15, 2024

Perhaps weights could be added into TrueSkillRating? I really don't know, if I was designing this API from the scratch I'd probably make some kind of builder struct which has a method to add a team and a method to calculate the ratings. The teams themselves would also be constructed using some other builder struct. I don't think that's a good design though, I just don't know how to do it better.

@atomflunder
Copy link
Owner Author

Perhaps weights could be added into TrueSkillRating?

Nah, weights aren't tied to your rating, but the match itself.

I really don't know, if I was designing this API from the scratch I'd probably make some kind of builder struct which has a method to add a team and a method to calculate the ratings. The teams themselves would also be constructed using some other builder struct. I don't think that's a good design though, I just don't know how to do it better.

I don't know either. I originally wanted to keep it simple, but I guess TrueSkill just is not simple 😅

@atomflunder
Copy link
Owner Author

atomflunder commented Oct 15, 2024

Maybe we could create 2 functions, one with weights and one without, and the one without just calls the weighted function with default weights?

But then again the API already has so many functions, I'm not sure if I wanna add another. Just kind of thinking out loud here.

I think generally a good "roadmap" would be to add this feature, and any other stuff that is missing to call this crate feature-complete and then only introduce a major API change with an eventual 1.0 release, if at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants