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

I0617 rescu sensors eval #626

Merged
merged 38 commits into from
Aug 23, 2024
Merged

I0617 rescu sensors eval #626

merged 38 commits into from
Aug 23, 2024

Conversation

gabrielwol
Copy link
Collaborator

@gabrielwol gabrielwol commented May 31, 2023

What this pull request accomplishes:

  • SQL to identify network wide outages and individual sensor VDS outages.

  • Identify RESCU detectors to repair this year during annual repair blitz. Committing this outdated work to vds schema is confusing and if committed to rescu it would never be seen again so I chose to delete it. We have a new dashboard being developed by Chris which identifies sensors to repair and supersedes this piece of the PR. I kept and documented the two views I developed which may be helpful for others.

Issue(s) this solves:

What, in particular, needs to reviewed:

What needs to be done by a sysadmin after this PR is merged

Nothing

@radumas
Copy link
Member

radumas commented May 31, 2023

can you rename the Data Validation folder to validation? The fluffer doesn't like the space, and I am allergic to capitalization.

@radumas radumas added the RESCU For VDS detector stations label Jun 12, 2023
@radumas radumas linked an issue Jun 12, 2023 that may be closed by this pull request
@radumas radumas requested a review from scann0n June 21, 2023 15:05
Copy link
Member

@radumas radumas left a comment

Choose a reason for hiding this comment

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

Nice work.

Mostly formatting changes on volumes/rescu/rescu_sensors_eval_readme.md and the recommendation to split up the content.

I haven't super wrapped my head around how this PR interacts with Sarah's we can try to merge that one first? And then I might have further specific suggestions about where things go in documentation land.

volumes/rescu/sql/create-view-network-outages.sql Outdated Show resolved Hide resolved
volumes/rescu/rescu_sensors_eval_readme.md Outdated Show resolved Hide resolved
volumes/rescu/rescu_sensors_eval_readme.md Outdated Show resolved Hide resolved
volumes/rescu/rescu_sensors_eval_readme.md Outdated Show resolved Hide resolved
volumes/rescu/sql/create-mat-view-individual-outages.sql Outdated Show resolved Hide resolved
Comment on lines 1 to 5
--this query can be used to identify detectors for repair requests.
--recommend opening in qgis and styling based on 'classify'. Look for geographical gaps with bad/inactive sensors.
--designate a set of `time_bins` (ie. current year) to get exact % active bins
--includes inactive sensors that don't have any records during the designated `time_bins`
--runs in 20s for 2023-01--2023-05
Copy link
Member

Choose a reason for hiding this comment

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

I think this could this be a mat view in the rescu schema, change the end dates to be now() plus some manipulation to include the last pulled date.

Copy link
Member

Choose a reason for hiding this comment

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

orrrrrrrrrr... it could be a function that returns a SET/Table, with inputs of start and end date. I don't think this is necessary to implement rn, but a thought.

volumes/rescu/rescu_sensors_eval_readme.md Outdated Show resolved Hide resolved
volumes/rescu/rescu_sensors_eval_readme.md Outdated Show resolved Hide resolved
@radumas
Copy link
Member

radumas commented Sep 27, 2023

So we're going to revisit this and find the work a new home within the new volumes/vds folder after #646 gets merged

@gabrielwol gabrielwol self-assigned this Sep 27, 2023
Copy link
Collaborator

@scann0n scann0n left a comment

Choose a reason for hiding this comment

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

Thanks for this gwolofs!

I found one smol typo in the readme (see inline comments) and there are some outstanding fluffs.

Organizationally speaking:

  • on bigdata - these detector mat views are too good to be hidden in gwolofs - have they been moved somewhere more public? I snooped on bigdata.vds but I did not see them there - are they somewhere else? Have they been superceded?
  • on bdit_data-sources - this work is saved to the rescu folder which is appropriate given the focus. Is there any desire to migrate these to the vds folder and purge the rescu folder?
    • If there is, I would just create a "rescu_checks" subfolder in vds and migrate your folder structure there (pending any @radumas objections).
    • I also think it would be fine to leave the rescu folder as-is

volumes/rescu/validation/readme.md Outdated Show resolved Hide resolved
@gabrielwol gabrielwol marked this pull request as draft June 25, 2024 17:39
@gabrielwol gabrielwol marked this pull request as ready for review July 30, 2024 20:34
@radumas
Copy link
Member

radumas commented Aug 20, 2024

Realize this wasn't introduced in this PR but the below bullets didn't render properly

- Data quality checks have not been implemented in this new schema. For examples of past work see:
- @scann0n did some work on identifying good date ranges for RESCU sensors based on volumes from days with all 96 15-minute bins present which is written up [here](https://github.com/CityofToronto/bdit_data-sources/blob/master/volumes/rescu/README.md#6--how-often-are-data-quality-assessment-processes-for-the-data-undertaken).
- @gabrielwol did work to identify periods of network wide or individual sensor outages on the RESCU network which is written up [here](https://github.com/CityofToronto/bdit_data-sources/blob/3ba3af5068e96191caffab524d42ae52fe7be7b2/volumes/rescu/README.md#1--are-there-known-data-gapsincomplete-data)

volumes/vds/readme.md Outdated Show resolved Hide resolved
Copy link
Member

@radumas radumas left a comment

Choose a reason for hiding this comment

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

SQL looks good. Main issue is markdown tables don't render properly.

Given the deletion of the broken sensor identification work because it is superseded, I identified some links that are likely going to rot when this PR is merged and the branch is deleted

volumes/vds/readme.md Outdated Show resolved Hide resolved
volumes/vds/readme.md Outdated Show resolved Hide resolved
volumes/vds/readme.md Outdated Show resolved Hide resolved
Copy link
Member

@radumas radumas left a comment

Choose a reason for hiding this comment

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

:meow_mage:
:party-porg:

@radumas radumas dismissed scann0n’s stale review August 23, 2024 15:57

stale review. Gabe has made substantial changes that make the requested changes irrelephant

@gabrielwol gabrielwol merged commit c096519 into master Aug 23, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RESCU For VDS detector stations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Identify RESCU VDS locations in need of repair
3 participants