Polish the 'ignored' logic / styling / tooltip

brendanheywood commented 7 years ago

https://www.thecrag.com/climbing/world/ascents/has/rating+ascent-date/with-route-gear-style/boulder/by/fede9/?sortby=rating,desc

Every single one of these ascents is ignored, and yet something must be counted somewhere because this guy is ranked 1st boulderer in the world. What gives? I think there is some edge case bug around the ascent date

Also I think the strike through for pages like this seems overly harsh, so I'll tone it down a little.

brendanheywood commented 7 years ago

before

after

scd commented 7 years ago

Yup, much better

brendanheywood commented 7 years ago

@scd just making sure you didn't miss the bit for you, that guy's top ascent is ignored which should be impossible. By definition I don't think the top ascent listed by rating should ever be ignored? They have only ticked it once

https://www.thecrag.com/climbing/world/ascents/has/rating+ascent-date/with-route-gear-style/boulder/by/fede9/?sortby=rating,desc

scd commented 7 years ago

I had missed it. I think there is something else going on and the ascent is not being ignored, but rather we are saying it is erroneously. I will look into this soon but not today or tomorrow

brendanheywood commented 7 years ago

Some lists of ascents like Adam's are way worse, like almost every single ascent of his is shown as ignored when it clearly isn't.

https://www.thecrag.com/ascents/with-route-gear-style/sport/by/concept/?sortby=rating,desc

His calculated current cpr stat is 3690 which I could understand generally if it has some delay and was slightly stale (but not in his particular case). But that seems to have very little correlation with his top ascent cpr which is 3717. The latter should be a live query and in theory perfect?

I'm the same: https://www.thecrag.com/ascents/with-route-gear-style/sport/by/brendanheywood/?sortby=rating,desc

In fact I've just tried half a dozen people and I can't actually find anyone who is good

scd commented 7 years ago

This is now hotfixed. It was label bug getting the wrong offset variable for the stored stat, which meant the returned power value was never decayed from when it was stored. That was fine last week as it takes a couple of days to decay a whole point, but this week we were seeing the systematic failure come to life.

All the other issues seem fine to me, I think you are just second guessing the maths because of the cross out misfiring. The values seem reasonable to me based on our configuration and processing. If you still disagree then we should walk through one example. Otherwise please close

brendanheywood commented 7 years ago

If you still disagree then we should walk through one example.

Ideally this should be in the article for everyone not just me. But lets do a walk here first for my account. I still don't get why my largest CPR for an ascent is 2903 but my stats cpr in the graph is lower at 2888. And worse the cpr value in the my account template data is not consistent either at 2914.

scd commented 7 years ago

Hmmm, I think there is a problem. It seems as though I might be decaying stuff on the graph using the wrong offset.

scd commented 7 years ago

After a bit of anguish, I think I know what is going on. There are a couple of issues leading to confusion.

Firstly the CPR presented on the timeline is the CPR at the end of the timeslice. For example if the time slice is at a month and you do a grade 28 ascent on the 15th, and a grade 21 ascent on the 31st then the ascent on the 15th is decayed to the 31st of the month when presenting the CPR. This is potentially really confusing, because I think people would expect the highest CPR for the time slice to be presented not the CPR at the end of the timeslice.

@brendanheywood if you think we need to make a change based on the above observation then we should separate this into another issue.

Secondly the graph defaults are a bit weird when drilling down. If you drill down on Brendan's cpr to to the day of his highest cpr ascent you eventually get to the following url

https://www.thecrag.com/ascents/with-route-gear-style/sport/view-between/2016-11-3+2016-11-9/by/brendanheywood/between/2016-11-6+2016-11-6?sortby=rating,desc

This links to the following iframe:

https://www.thecrag.com/ascents/aggregate/graph-sport-cpr/with-route-gear-style/sport/view-between/2016-11-6+2016-11-6/view-view-between/2016-11-3+2016-11-9/by/brendanheywood/?embed=html

(note there is teh view-view-between extra field, but it does not effect the result).

Because there is only one day in the list, the backend chooses a period of a quarter. This means the first issue effects the CPR display. If I then force this to be a day period we are now getting closer.

https://www.thecrag.com/ascents/aggregate/graph-sport-cpr/with-route-gear-style/sport/view-between/2016-11-6+2016-11-6/by/brendanheywood/?period=day

But if we then ignore previous ascents we get:

https://www.thecrag.com/ascents/aggregate/graph-sport-cpr/with-route-gear-style/sport/view-between/2016-11-6+2016-11-6/between/2016-11-6+2016-11-6/by/brendanheywood/?period=day

The CPR of 2948 now matches the your ascent breakdown.

If we make a correction for the first issue then the second issue will disappear.

There is a third issue which I will right up separately. We don't have a good definition for how to handle the final shift back.

brendanheywood commented 7 years ago

yeah not surprising that there was a few moving parts here

Firstly the CPR presented on the timeline is the CPR at the end of the timeslice. For example if the time slice is at a month and you do a grade 28 ascent on the 15th, and a grade 21 ascent on the 31st then the ascent on the 15th is decayed to the 31st of the month when presenting the CPR. This is potentially really confusing, because I think people would expect the highest CPR for the time slice to be presented not the CPR at the end of the time slice.

I think we should split this into two parts:

[ ] 1) is whether we use endEpoch or the max for that period. I agree max is what most people would expect. We should fix it but I don't think it's the crux of this issue. The max of the max'es (ie the black cpr number) isn't directly compared to anything else. As we've commented before it's not directly related to anything, not even the all time undecayed cpr, so it's not inconsistent with anything except when you zoom in with itself.
[ ] 2) The main crux is a similar idea but applied only to the last time slice which only has partial data up to today. The final number on the graph (black if only 1 is shown or they gray if 2 are shown) should always be your current cpr and perfectly match your cpr elsewhere like the world ranking and the account page template data (assuming stats are up to date). Even if you have a little bit of decay and your highest for that period is a tad higher. I think purely for the last time slice we should add an extra chunk of data which is data for today as opposed to endEpoch. We still need to know the end epoch for calculating the width and scale correctly.
[x] we also need to fix the links in the svg to strip out the view- and view-view- url bug

scd commented 7 years ago

The main crux is a similar idea but applied only to the last time slice which only has partial data up to today. The final number on the graph (black if only 1 is shown or they gray if 2 are shown) should always be your current cpr and perfectly match your cpr elsewhere like the world ranking and the account page template data (assuming stats are up to date).

Sort of. This is correct if the last time slice includes the current date, otherwise it is just like any other time slice.

I agree we should show your actual CPR if the last time slice includes current date.

How do you want it? The template data already has account level CPR in the template data.

"currentRating": {

    "context": {
        "contextID": "7516384",
        "contextName": "Australian (Ewbanks ...)",
        "systems": [
            "AU",
            "BLDV",
            "AUAID",
            "STAR",
            "PROT"
        ],
        "context": "AU"
    },
    "sport": {
        "system": "AU",
        "decayGrade": "24",
        "decay": 2914,
        "totalGrade": "26",
        "systemName": "Ewbanks",
        "total": 3157
    },
    "trad": {
        "system": "AU",
        "decayGrade": "22",
        "decay": 2576,
        "totalGrade": "24",
        "systemName": "Ewbanks",
        "total": 2836
    },
    "boulder": {
        "system": "BLDV",
        "decayGrade": "V6",
        "decay": 3233,
        "totalGrade": "V8",
        "systemName": "V-Scale",
        "total": 3540
    }

brendanheywood commented 7 years ago

[x] I've just fixed the _parent target bug too so all these drill downs just load in the same tab instead of spawning more and more tabs.

I've also fixed the between vs view-between bugs

brendanheywood commented 7 years ago

I should be in the last time slice data, this has almost nothing to do with the account level data. This data could be for a node not world, and it could also be for a group of people or a whole node.

I think I want extra data in the last slice like this:

"cpr": {
"band": 4,
"internalGrade": 325.4,
"grade": "V6",
"decayOffset": 17168,
"rating": 3270,
"currentRating": 3278, <------------ new
"currentEpoch": 1234567, <------------ new
"systems": [],
"ascentCount": 0,
"power": 22141098878489600
},

[ ] there is also another small bug which I'm not sure what to do about or wether it matters. We are hiding the saw tooth effect of the real cpr. When you zoom in depending on how you cut the cut tooth curve this is partially exposed. So on a high zoom level it looks like your cpr is slowly going down, but when you zoom in it's already down so can be going back up.

scd commented 7 years ago

I have added the following three variables to the cpr template data item:

"currentDO": 17258
"currentEpoch": 1491156116
"currentRating": 2915

Note that currentDO is the day index which is the variable used to calculate the decay. Are you just using this for final presentation, or do we need to make sure we have conversions to grade, etc based on this rating?

scd commented 7 years ago

Also I have added the peak power stuff, for just the cpr data item:

"peakDO": 13296
"peakRating": 2978

I am not sure how important this is. One thing that comes to mind as well as the drill down, is if somebody is looking at their performance over years they would want to visually see their peaks. If somebody climbs their hardest climb in January and the chart is has a period of a year then by the end of the year this has dropped almost 100 points if it is not backed up with another hard ascent later on in the year?

I am also happy to leave this to gather further feedback, because it opens up other questions. Should we be doing the same with the other tick type cpr curves?

brendanheywood commented 7 years ago

Ok so I have fixed up the currentCpr in the graph, but I'm still getting inconsistent numbers:

The timeline is giving me 3237 as my current cpr, by my account stats is saying 3228

scd commented 7 years ago

There are a couple of things we need to look at hear.

currentRating in the cpr timeline is calculated dynamically using the timeline based on today's date. Theoretically it should be the same, but it is not in this case. We could use the rating in the statistics field so that errors are hidden. There are several reasons why I don't want to do that, but I just want to confirm we are on the same page.

So why is it different. Have you been doing any tick testing on dev? If you have added ticks to your dev then this will be in your timeline but not your stats.

If not then this is concerning because they are a long way from each other, which means there is a bug somewhere. Let's see how it goes on prod after the release and I will investigate further if there continues to be a significant miss match.

brendanheywood commented 7 years ago

Yeah I haven't ticked on dev and asumme the stats process should be up to date so it smells like a bug to me.

brendanheywood commented 7 years ago

Ok so I think the last remaining action here is to add some slice peak tooltip stuff which I've now done:

This issue has diverged heaps from the original one, so I'm closing this. If there is still an issue later with the two cpr's lets make a new one

theCrag / website

Polish the 'ignored' logic / styling / tooltip #2678