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

make fuzz vary by rating and over the season #344

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions src/worker/core/player/fuzzRating.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,52 @@
import { PHASE } from "../../../common";
import { g, helpers } from "../../util";

const fuzzRating = (rating: number, fuzz: number): number => {
// Generate plus minus from ovr
const getO2PM = (ovr: number) => {
const iv = [72.7, 7.28 * 2, 18.1 / 2];
const minVal = -2.021381747657189;
return minVal + iv[1] / (1 + Math.exp(-(ovr - iv[0]) / iv[2]));
};
// get ovr from plus minus
const getPM2O = (pm: number) => {
const iv = [72.7, 7.28 * 2, 18.1 / 2];
const minVal = -2.021381747657189;
//clip plus minus, don't send players to 0 lol
const pmC = Math.min(iv[1] + minVal, Math.max(minVal + 0.11, pm));
// invert the sigmoid above
const ovr = iv[0] - iv[2] * Math.log(iv[1] / (pmC - minVal) - 1);
// clip overall
const ovrC = Math.min(100, Math.max(0, ovr));

return ovrC;
};

const fuzzRating = (rating: number, fuzz: number, isDraft = false): number => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would need to accept season as a parameter too, right? Otherwise fuzz for past season ratings would be constantly oscillating.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could default to g.get("season") since in many places it's only the current season, probably.

Copy link
Contributor Author

@nicidob nicidob Jan 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you don't store fuzz per season, do you? That'd need to be added? At which point, better remove the storage for fuzz entirely and make it something you'd want to do deterministically on the fly then (hash pid, season and a per-league salt as the seed to an RNG).

// Turn off fuzz in multi team mode, because it doesn't have any meaning there in its current form. The check for
// existence of variables is because this is sometimes called in league upgrade code when g is not available and
// would be difficult to make available due to Firefox promise/IDB/worker issues.
if (
(g.hasOwnProperty("userTids") && g.get("userTids").length > 1) ||
g.get("godMode")
) {
fuzz = 0;
return rating;
}

return Math.round(helpers.bound(rating + fuzz, 0, 100));
let frac_left = 1;

// this shouldn't happen for draft prospects.
if (!isDraft) {
// would love to use number of days;
//const numDays = season.getDaysLeftSchedule();
//const numTotal = g.get("numGames");

frac_left = g.get("phase") >= PHASE.PLAYOFFS ? 0 : 1;
}

// fuzz is -5 to 5. Let's assume it's really -1 to 1 in +/- terms
const fuzzed = getPM2O(getO2PM(rating) + (frac_left * fuzz) / 5);

return Math.round(helpers.bound(fuzzed, 0, 100));
};

export default fuzzRating;
9 changes: 5 additions & 4 deletions src/worker/db/getCopies/playersPlus.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,11 @@ describe("worker/db/getCopies/playersPlus", () => {
}

// This will break if ovr + fuzz is over 100 (should check bounds), but that never happens in practice
assert.strictEqual(
pf.ratings.ovr,
Math.round(p.ratings[1].ovr + p.ratings[1].fuzz),
);
// fuzz is not this simple anymore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should just be deleted rather than commented out

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking was I should add tests. Check for decent bounds. Check that the +/- functions behave correctly. I don't think losing tests make sense, especially when the code just got more complicated. I should just write some additional replacement test.

//assert.strictEqual(
// pf.ratings.ovr,
// Math.round(p.ratings[1].ovr + p.ratings[1].fuzz),
//);
});

test("return stats from previous season if options.oldStats is true and current season has no stats record", async () => {
Expand Down