OpenBeta / sandbag

JS utilities for working with climbing grades
MIT License
24 stars 17 forks source link

Ambiguous handling of non-letter'd YDS grades above 5.9 #137

Closed johncalvinroberts closed 1 year ago

johncalvinroberts commented 1 year ago

Problem

Using the YosemiteDecimal scale, it seems like there is some unclear/ambiguous handling of what isValid vs. what can be parsed into a score from a grade.

Specifically, take the example "5.11".

YosemiteDecimal.isType will tell us this is a valid grade for YDS, and YosemiteDecimal.getScore will return a tuple representing the difficulty score on a scale of 0-100. However, the value returned is not an "integer", it returns [69.5, 71.5]. This is where the bug actually manifests itself, since we expect the value of getScore to be a valid index of the routes data array) when working with freeclimbing routes, e.g., here and here.

So, that is to say, the value returned by getScore is not a valid representation of a grade. This creates an issue if we want to then take the return value of getScore, store it in memory, then at some arbitrary point in time convert that back into a grade using YosemiteDecimal.getGrade. It will break because the return value of getScore is not a valid index.

We can see a demo of this bug in action in this stackblitz link here. Check the console to see the error message --

Error: Cannot read properties of undefined (reading 'yds')

So, tldr: the YosemiteDecimal module is ambiguous in its handling of grades with no letter above 5.9, e.g., "5.10", "5.11", "5.12", etc. No issue with "5.11a", and no issue with "5.9". The isType func says this is a valid grade, but it doesn't exist in the routes file, and doesn't return a valid score when parsed with getScore.

Proposed Solution

Possible solutions:

vnugent commented 1 year ago

I'm seeing similar issue when using getScore() to build charts. (live profile)


Screenshot 2023-07-24 at 5 56 27 AM
musoke commented 1 year ago

@johncalvinroberts, thank you for reporting this!

don't treat non-letter'd grades above 5.9 as actual grades, respecting the contents of routes.json and also what seems to be the prevailing convention -- 5.11 is not a grade value, but a range. Again, not sure about my wording here :smile:

Do you mean that 5.11 should correspond to the whole range 5.11a-5.11d? If so, I would probably agree. Need to look at the logic in here again, but I think the score has been converted like a slash grade. You'll likely find similar issues when converting 5.11c/d to score and back.

I would also say that getGrade should be the exact inverse of getScale.

musoke commented 1 year ago

Another possible solution: round in getScore.

johncalvinroberts commented 1 year ago

Do you mean that 5.11 should correspond to the whole range 5.11a-5.11d? If so, I would probably agree.

I was more suggesting that basically 5.11 not be treated as a grade in the strict sense, e.g., YosemiteDecimal.isType("5.11") would return false, and instead the concept of "5.11 is a range of grades" just existing in the human world but not in the sandbag world. Unless -- does sandbag support ranges like that already?

Also, FWIW: I don't still believe this, I think it makes more sense for 5.11 to default to a valid grade on the scale 😄

musoke commented 1 year ago

I was more suggesting that basically 5.11 not be treated as a grade in the strict sense, e.g., YosemiteDecimal.isType("5.11") would return false, and instead the concept of "5.11 is a range of grades" just existing in the human world but not in the sandbag world.

Also, FWIW: I don't still believe this, I think it makes more sense for 5.11 to default to a valid grade on the scale 😄

I would say that sandbag treats all grades as a range of scores and that anything that can be converted to a score is a valid grade. The grades appearing in the tables are just an implementation detail. They are a sufficiently fine-grained representation of scores to calculate scores of most valid grades by, for example, saying that the score range for 5.11a/b is from the middle of 5.11a to the middle of 5.11b.

With that context, would you say that treating 5.11 as the union of scores ranges for 5.11a-5.11d is correct?

Unless -- does sandbag support ranges like that already?

Not quite, but it should. There are some grades that could be converted more precisely with ranges in getScore. For example, in the current table UIAA II covers 6 French grades and 4 Ewbank grades. Converting should output a range - French 2a-2c+ or Ewbank 5-8.

On Tue, Jul 25, 2023, 09:29 John Roberts @.***> wrote:

Do you mean that 5.11 should correspond to the whole range 5.11a-5.11d? If so, I would probably agree.

I was more suggesting that basically 5.11 not be treated as a grade in the strict sense, e.g., YosemiteDecimal.isType("5.11") would return false, and instead the concept of "5.11 is a range of grades" just existing in the human world but not in the sandbag world. Unless -- does sandbag support ranges like that already?

Also, FWIW: I don't still believe this, I think it makes more sense for 5.11 to default to a valid grade on the scale 😄

— Reply to this email directly, view it on GitHub https://github.com/OpenBeta/sandbag/issues/137#issuecomment-1649851019, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD7ET7DULQVL4WXANJESN3LXR7C47ANCNFSM6AAAAAA2UJMTK4 . You are receiving this because you commented.Message ID: @.***>

johncalvinroberts commented 1 year ago

With that context, would you say that treating 5.11 as the union of scores ranges for 5.11a-5.11d is correct?

TBH, I'm not sure what the prevailing convention is in the climbing world. If you see 5.11 in a physical paperback guidebook, does that mean 5.11a? Or does that mean 5.11b/c? Or does that mean "this route could be anywhere between 5.11a and 5.11d in difficulty but we haven't specified to that level of precision"?

musoke commented 1 year ago

If I see 5.11 in a guidebook I assume either

If I see 5.11 in someone's spray, I assume 5.11a.

vnugent commented 1 year ago

In the original implementation (which only supported YDS) the order was something like this:

It's inline with this chart (Full discussion )

yds-grades

musoke commented 1 year ago

Thank you @vnugent! Those interpretations make sense and I think we should continue to follow them unless we have good reason not to. We should write this down somewhere, maybe in doc strings with doc tests of the specific cases.

johncalvinroberts commented 1 year ago

So, is that a conclusion? The prevailing convention in the data source is 5.11 = 5.11b/c?

If so, then I think rounding with Math.floor is a solution (regardless of where it's done, getScore vs. getGrade, etc).

Not trying to force a decision, just trying to follow along 😄

musoke commented 1 year ago

@vnugent, there is a slight ambiguity in your comment:

I have taken a look at the code in question, and neither interpretation is implemented correctly.

  1. is not true because getScore(5.11) = [69.5, 71.5] rather than [69, 72].
  2. is not true because getScore(5.11b/c) = [69, 70] = getScore(5.11b)

I think there are at least three interacting issues here

  1. How is a YDS grade with number greater than 9 and no letter (e.g. 5.11, 5.11- or 5.11+) converted a score range?
  2. Are non-integer scores allowed? If so how are they converted to grades? I think @johncalvinroberts's proposed fix addresses this well. Scores received by getGrade should definitely be sanitized. It might be good to do this in both getScore and getGrade.
  3. YDS Slash grades are broken (see https://github.com/OpenBeta/sandbag/issues/142)
vnugent commented 1 year ago

the table you showed seems to say 5.11 = 5.11b-c (ie a range including all of 5.11b and 5.11c)

I believe 5.11b/c means it's harder than 11b but easier than 11c. It can not have the range including both 11b and 11c. Another word

How is a YDS grade with number greater than 9 and no letter (e.g. 5.11, 5.11- or 5.11+) converted a score range?

I think this table should clear up:

10a = 5.10- 10a/b 10b 10b/c = 5.10 10c 10c/d 10d = 5.10+

Are non-integer scores allowed? If so how are they converted to grades? I think @johncalvinroberts's proposed fix addresses this well. Scores received by getGrade should definitely be sanitized. It might be good to do this in both getScore and getGrade.

I think we shouldn't allow non-integer scores.

musoke commented 1 year ago

Ok, I think @johncalvinroberts's #138 will take care of everything here.