868 miovision find gaps bugs#869
Draft
gabrielwol wants to merge 4 commits intomasterfrom
Draft
Conversation
remove 15 minute buffer + change to adding artificial point at each day
This comment was marked as outdated.
This comment was marked as outdated.
Collaborator
Author
|
Close #1024 before this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this pull request accomplishes:
fixes undesired behaviours in find_gaps:
unacceptable_gapsandvolumes_15min_mvt. This was the source of error for 3 of the gaps identified in the original issue.generate_seriesinstead of justvalues(start_date, end_date)so they get broken up by day. To fix this we can just edit existing multiday gaps.daily_intersectionsto fullintersectionscross join. Issue: we currently have full intersection x days of just zeros involume_15min_mvtwhen they should be nulls.Issue(s) this solves:
What, in particular, needs to reviewed:
What needs to be done by a sysadmin after this PR is merged
There are a bunch of cases we will need to re-run aggregation for. I think we can do all of these more surgically instead of updating all of unacceptable_gaps, which is very slow.
Case 1: '15 minute lookback'. 60 gaps which should be re-run to remove midnight overlap. Should be fast enough to re-run all these aggregations based on my testing (7s for aggregating 1 day, 1 intersection):
Volumes_15min_mvtunaffected.Update: ✅
volumes_15min_mvtcorresponding to full day outages where we should have nulls instead of zeros. Set both v15 tables to null and add tounnacceptable_gaps. volumes_daily is unaffected because it sums from volumes table directly.Updated ✅
UPDATE 5875075: