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

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

Closed
johncalvinroberts opened this issue Jul 23, 2023 · 13 comments
Closed

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

johncalvinroberts opened this issue Jul 23, 2023 · 13 comments
Labels
bug Something isn't working

Comments

@johncalvinroberts
Copy link
Contributor

johncalvinroberts commented Jul 23, 2023

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:

  • You could consider defaulting a non-letter'd grade above 5.9 as the lowest grade in that level. Not sure if my wording is correct here! In other words, though, an argument of passed to getScore as 5.11 would be directly treated as 5.11a, return [67, 68] instead of [67.5, 71.5]
  • Instead of defaulting to the lowest grade in getScore, we could consider rounding the values at some point in getGrade to avoid accessing non-existant indexes of routes.
  • IMO, the most straightforward solution: 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 😄
@vnugent
Copy link
Contributor

vnugent commented Jul 24, 2023

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


Screenshot 2023-07-24 at 5 56 27 AM

@musoke musoke added the bug Something isn't working label Jul 24, 2023
@musoke
Copy link
Collaborator

musoke commented Jul 24, 2023

@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 😄

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
Copy link
Collaborator

musoke commented Jul 24, 2023

Another possible solution: round in getScore.

@johncalvinroberts
Copy link
Contributor Author

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
Copy link
Collaborator

musoke commented Jul 25, 2023 via email

@johncalvinroberts
Copy link
Contributor Author

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
Copy link
Collaborator

musoke commented Jul 25, 2023

If I see 5.11 in a guidebook I assume either

  • 5.11a-5.11d
  • 5.11b/c, with 5.11-=5.11a-b, 5.11+=5.11c-d

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

@vnugent
Copy link
Contributor

vnugent commented Jul 25, 2023

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

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

It's inline with this chart (Full discussion )

yds-grades

@musoke
Copy link
Collaborator

musoke commented Jul 25, 2023

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
Copy link
Contributor Author

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
Copy link
Collaborator

musoke commented Jul 26, 2023

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

  • the table you showed seems to say 5.11 = 5.11b-c (ie a range including all of 5.11b and 5.11c)
  • the text you wrote says 5.11 = 5.11b/c. We have been treating slash grades as intermediate grades, with a range from the middle of the lower to middle of the upper.

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 YDS slash grades broken #142)

@vnugent
Copy link
Contributor

vnugent commented Jul 30, 2023

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

  • getScore('5.11b/c') == getScore('5.11')
  • getScore('5.11b') < getScore('5.11') < getScore('5.11c')

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.

johncalvinroberts added a commit to johncalvinroberts/sandbag that referenced this issue Jul 30, 2023
@musoke
Copy link
Collaborator

musoke commented Jul 31, 2023

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

@musoke musoke closed this as completed in ab39cd9 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

3 participants