Improves Admin Analytics, Adds Choropleth #692

misaugstad · 2017-06-19T04:10:34Z

On admin overview page

The coverage table now has "rate" as a column instead of a row

On admin analytics page:

neighborhood completion choropleth added
DC coverage progress area chart added
daily label counts chart updated to include all data, not just past month
severity rating histograms for each label type added
onboarding completion time histogram updated to be more informative
coverage % by neighborhood chart's axes switched so neighborhood names are legible
neighborhood coverage % histogram added
completed missions histogram removed
neighborhood total distance covered chart removed

Other:

region_completion table added to database

Resolves #691
Resolves #622
Resolves #676
Resolves #697
Partially Resolves #342

…Webpage into 342-admin-improve-analytics-graphs

…tead of audits

…analytics-graphs Conflicts: public/javascripts/Admin/src/Admin.js

manaswisaha · 2017-06-20T13:51:46Z

Meta point: Another rule of thumb for PRs that I am realizing now is that we should aim to keep PRs as small as possible. Just address a single bug or small number of related bugs. Currently, this PR is really huge to review with a lot of changes. Would take quite some time to review the code.

manaswisaha · 2017-06-20T13:52:23Z

I'll create a doc on creating PRs after relaunch and add it to the wiki of this repo.

jonfroehlich · 2017-06-20T13:54:37Z

At this point, I think you can do a light code review of admin-centric stuff. We can then do a more heavyweight review after relaunch (which I do think is important because we want to make sure not just that the admin interface contains the analytics that we need and is coded well but that the underlying calculations are correct).

misaugstad · 2017-06-20T14:06:32Z

I agree with @jonfroehlich to keep the code review light. Especially since there are still changes to be made, the code needs to be cleaned up a little bit as well, and there is some time-sensitivity to getting the backend stuff integrated for testing purposes.

I also agree that PRs should be as small as possible. The problem here is that it is meant to address an issue, but the issue is huge; the issue has over 20 charts to be added/modified that are mentioned, so should I be making a dozen PRs that all partially address that one issue..? I don't think that the issue should be split up, as that would be hard to manage, but does it make sense to split up the PRs?

manaswisaha · 2017-06-20T16:04:50Z

@misaugstad

Feedback:
I tested the neighborhoodCompletionRate code by doing couple of missions both as an anonymous and a registered user. It updates properly. Other comments:

I left a few comments in the PR to address.
About the region_completion table,
- it initializes with audited distance for region ids from 195 - 373. What happened to the ids before that? Are those invalid?
- The distance captured in the table is in feet?
There are print statements left to be removed within Scala code:

manaswisaha · 2017-06-20T16:05:52Z

Also, didn't comment on the cosmetic changes that are needed (e.g. spacing between graphs, graphs sticking to the window border). I assume those will taken care of in the next round of updates.

jonfroehlich · 2017-06-20T16:09:36Z

I think we should likely use SI metrics on backend and can change to imperial system for front facing UI (as we currently do). But it's obviously not worth changing backend stuff if it's all in imperial... in general, however, as scientists we should be using scientific metrics (SI!).

…

On Tue, Jun 20, 2017 at 12:05 PM, Manaswi Saha ***@***.***> wrote: Also, didn't comment on the cosmetic changes that are needed (e.g. spacing between graphs, graphs sticking to the window border). I assume those will taken care of in the next round of updates. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#692 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi-9Zzc_aKb5HOi5MrRb9B5uVhpdWG7ks5sF-3hgaJpZM4N9vGp> .

-- Jon Froehlich Assistant Professor Computer Science University of Maryland, College Park http://www.cs.umd.edu/~jonf/ @jonfroehlich <https://twitter.com/jonfroehlich> - Twitter

manaswisaha · 2017-06-20T16:22:54Z

@misaugstad Actually, I was wrong. This doesn't work as expected. I did some more testing. The code within updateAuditTaskCompleteness() --> RegionCompletionTable.updateAuditedDistance(auditTask.streetEdgeId) is called for multiple audits of the same street. I audited the street multiple times to check if the table is being updated again or not, and it does.

if (updatedCount == 1) {
        print("Updated only once " + auditTask.streetEdgeId)
        RegionCompletionTable.updateAuditedDistance(auditTask.streetEdgeId)
      }
    } else if (incomplete.isDefined && incomplete.get.issueDescription == "GSVNotAvailable") {
      // If the user skipped with `GSVNotAvailable`, mark the task as completed and increment the task completion
      AuditTaskTable.updateCompleted(auditTaskId, completed=true)
      val updatedCount: Int = StreetEdgeAssignmentCountTable.incrementCompletion(auditTask.streetEdgeId) // Increment task completion
      // if this was the first completed audit of this street edge, increase total audited distance of that region.
      if (updatedCount == 1) {
        RegionCompletionTable.updateAuditedDistance(auditTask.streetEdgeId)
      }
    }

misaugstad · 2017-06-20T16:27:40Z

I left a few comments in the PR to address.

Not sure what you mean, is this not the PR?

About the region_completion table, it initializes with audited distance for region ids from 195 - 373. What happened to the ids before that? Are those invalid?

The ones before that have the deleted flag set to true.

The distance captured in the table is in feet?

I am pretty sure they are in meters. Whatever those "geom.transform(26918).length" calls give.

There are print statements left to be removed within Scala code:

I will find and eliminate.

manaswisaha · 2017-06-20T16:30:36Z

Not sure what you mean, is this not the PR?

Yes, I meant within the code on github's website. Scroll up or go to Files Changed tab to see those. Looks like this:

jonfroehlich · 2017-06-20T16:40:49Z

When you guys find code like this that is poorly documented and/or non-sensible:

I am pretty sure they are in meters. Whatever those "geom.transform(26918).length" calls give.

I would strongly prefer that you fix the documentation and/or improve code clarity. We need to know what metrics we are using (see http://articles.latimes.com/1999/oct/01/news/mn-17288).

misaugstad · 2017-06-20T16:43:58Z

I audited the street multiple times to check if the table is being updated again or not, and it does.

This should be fixed now.

misaugstad · 2017-06-20T16:58:29Z

Yes, I meant within the code on github's website. Scroll up or go to Files Changed tab to see those. Looks like this:

I scrolled up and clicked on Files changed but I am not seeing any comments

manaswisaha

Reviewed

manaswisaha · 2017-06-20T13:52:54Z

app/controllers/AdminController.scala

-      val CRSEpsg26918 = CRS.decode("epsg:26918")
-      val transform = CRS.findMathTransform(CRSEpsg4326, CRSEpsg26918)
-
+    RegionCompletionTable.initializeRegionCompletionTable()


Is the table initialized each time the function is called?

Oh I see if (regionCompletions.length.run == 0) line in RegionCompletionTable. Could you confirm if this prevents it from running from than once?

Yes I had confirmed that this didn't run multiple times using the magic of print statements. You also know that it isn't because the call takes >10 seconds to run, but there are never any 10 second lags when calling the getNeighborhoodCompletionRate after the first call.

manaswisaha · 2017-06-20T14:36:41Z

app/models/audit/AuditTaskTable.scala

@@ -112,7 +112,7 @@ object AuditTaskTable {
  def auditCounts: List[AuditCountPerDay] = db.withSession { implicit session =>
    val selectAuditCountQuery =  Q.queryNA[(String, Int)](
      """SELECT calendar_date::date, COUNT(audit_task_id) FROM (SELECT  current_date - (n || ' day')::INTERVAL AS calendar_date
-        |FROM    generate_series(0, 30) n) AS calendar
+        |FROM    generate_series(0, current_date - '11/17/2015') n) AS calendar


11/17/2015 - What does this date refer to?

Good question! I am also wondering that. It appears to be maybe the day after the audit_task_table was first created or something, because up until 11/16/2015 there are no audits at all then on 11/16/2015 there is an absurdly large number of audits, then the data starts looking normal after that.

In summary: 2 days before that date, there are no audits. The day before that date, the data is not reliable. So this is the date where reliable data begins.

Got it. Could you add a comment in the code for this?

manaswisaha · 2017-06-20T14:37:13Z

app/models/label/LabelTable.scala

@@ -467,7 +467,7 @@ object LabelTable {
  def selectLabelCountsPerDay: List[LabelCountPerDay] = db.withSession { implicit session =>
    val selectAuditCountQuery =  Q.queryNA[(String, Int)](
      """SELECT calendar_date::date, COUNT(label_id) FROM (SELECT  current_date - (n || ' day')::INTERVAL AS calendar_date
-        |FROM    generate_series(0, 30) n) AS calendar
+        |FROM    generate_series(0, current_date - '11/17/2015') n) AS calendar


Same comment as above about the hard coded date

Same response :)

… is outro

…-time-graph Fix onboarding time graph

misaugstad added 30 commits June 6, 2017 14:41

changed onboarding completion units from sec->min

90f2185

Merge branch 'develop' of https://github.com/ProjectSidewalk/Sidewalk…

f1d1de4

…Webpage into 342-admin-improve-analytics-graphs

landing and developer pages, DC completion rate now uses distance ins…

3b2f9c7

…tead of audits

cleaned up admin coverage table

b012109

replaced neighborhood statistics charts on admin page

0672367

adds chart for DC completion rate over time

737949a

replaced completed missions hist w/ severity rates hists

d1cb0fa

Merge remote-tracking branch 'origin/develop' into 342-admin-improve-…

378c539

…analytics-graphs Conflicts: public/javascripts/Admin/src/Admin.js

added neighborhood completion choropleth to admin analytics

224c14a

only query completion rate once, computute rate in controller

31dc1b8

added popup message to choropleth

af01525

added a few different popup dialogs

6b93139

fixed up severity rating histograms

161e587

widened DC completion graph

2b091a5

reverted DC completion graph back to area from line

18aa061

fixed axis titles for severity histograms

eb1a3cd

adjusted size of severity histograms

3299e46

label counts are now retrieved back to 2015

8a44d43

converted daily label count chart to vega-lite

87686c1

replaced onboarding completion time histogram

01c27d9

added new RegionCompletion table

7b0bbc6

added code to initialize the region_completion table

c02f3d4

region_completion table now updates after each street audit

ef546a8

new table is now used on for graphs on admin page, much faster

3bd0fd0

removed code that is no longer being used

331b28b

removed more code that is not in use

9b694d0

updated popup text on choropleth

5293d74

removed unused helper function

2668910

added legend to choropleth

4524029

removed commented code

ba63cdc

misaugstad added 2 commits June 20, 2017 12:41

removed print statements

7461cd2

fixed bug where repeated audits are both added to completed dist

edc3bd3

manaswisaha requested changes Jun 20, 2017

View reviewed changes

adevdash and others added 4 commits June 20, 2017 13:35

Onboarding completion time now checks that last onboarding transition…

b924d72

… is outro

Removed console.log

145d9bf

added explanatory comments

1dea7ed

Merge pull request #717 from ProjectSidewalk/716-admin-fix-onboarding…

e5fff69

…-time-graph Fix onboarding time graph

manaswisaha mentioned this pull request Jun 20, 2017

Checklist: v2 Relaunch Testing #687

Closed

41 tasks

manaswisaha approved these changes Jun 20, 2017

View reviewed changes

manaswisaha merged commit 3399161 into develop Jun 20, 2017

manaswisaha removed the pull-request-submitted label Jun 20, 2017

This was referenced Jun 21, 2017

Prevent users from auditing a completed neighborhood #691

Closed

Landing page: % of DC mapped uses audits, not distance #676

Closed

This was referenced Sep 17, 2017

Admin Interface: Coverage Rate per Neighborhood Graph #412

Closed

As a Potentially Motivating Feature... Show Map and Completion Rates of DC Neighborhoods on Frontpage #319

Closed

misaugstad self-assigned this Mar 2, 2019

misaugstad deleted the 342-admin-improve-analytics-graphs branch March 25, 2020 21:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improves Admin Analytics, Adds Choropleth #692

Improves Admin Analytics, Adds Choropleth #692

misaugstad commented Jun 19, 2017 •

edited

Loading

manaswisaha commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

jonfroehlich commented Jun 20, 2017

misaugstad commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

jonfroehlich commented Jun 20, 2017 via email

manaswisaha commented Jun 20, 2017

misaugstad commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

jonfroehlich commented Jun 20, 2017

misaugstad commented Jun 20, 2017

misaugstad commented Jun 20, 2017

manaswisaha left a comment

manaswisaha Jun 20, 2017

manaswisaha Jun 20, 2017

misaugstad Jun 20, 2017

manaswisaha Jun 20, 2017

misaugstad Jun 20, 2017

manaswisaha Jun 20, 2017

manaswisaha Jun 20, 2017

misaugstad Jun 20, 2017

Improves Admin Analytics, Adds Choropleth #692

Improves Admin Analytics, Adds Choropleth #692

Conversation

misaugstad commented Jun 19, 2017 • edited Loading

manaswisaha commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

jonfroehlich commented Jun 20, 2017

misaugstad commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

jonfroehlich commented Jun 20, 2017 via email

manaswisaha commented Jun 20, 2017

misaugstad commented Jun 20, 2017

manaswisaha commented Jun 20, 2017

jonfroehlich commented Jun 20, 2017

misaugstad commented Jun 20, 2017

misaugstad commented Jun 20, 2017

manaswisaha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

misaugstad commented Jun 19, 2017 •

edited

Loading