Time varying conversions #1546
Time varying conversions #1546
Conversation
…ine function that uses view, also all of Wendy's changes
…nit_v2 function to use said view.
…aily_readings_unit view
…urly_readings_unit view
…er_hourly_readings_unit view
…dings_unit, altered cik table to reflect previous groups work with time varying conversion, added 3rd version of meter line function based on previous groups implementaion of time varying conversions for testing purposes
…ed into meter_hourly_readings_unit view
…_unit implementing CIK table
Currently read-only. Uses fake data, as the Weeks API is not yet implemented.
…_readings_unit so that it no that it no longer has to apply the conversions it self.
… in navbar to "Patterns - Weekly"
* CRUD operations working with back-end * client-side validation * delete confirmation * Spanish and French translations
Omit `id` from the default week values model
docker-compose should not be ignored and should not have to be altered for our time-varying conversion work
…me varying conversions for raw readings.
…y works under the restriction that only 1 conversion is applied to an hourly reading.
…o longer need to apply conversions
- Fixed a long-standing bug where inserting cik into the DB used forEach with async function but that is not allowed. Converts to for/of loop so all okay. - Changes meter refresh model functions to use new views. - Changed compare meter to make sure range is within the hour as it did before time varying. It now matches group. - Changed shrink for meters by day to use new view. - Placed group views after meter views since they use them. - Fixed tests to use new views. Some need modification. to properly handle getting data. - Misc. formatting.
…_graphic_unit_id in graphing functions, removed index on old view that was cuasing an error
huss
left a comment
huss
left a comment
There was a problem hiding this comment.
Thanks to @tdthompson650 for doing this work. Overall, it works well. I have created some examples of expected results and manual testing indicates they are all correct so far. In the future these and others will be integrated into OED testing for more complete checks. There are some comments in the code to consider. Here are a few that could not easily be put into the code:
- There are a couple of merge conflicts.
- This branch seems behind timeVary and also includes code from the view PR. I'm will to work on this issue but wanted to note it. I have not reviewed the code from the other PR as it is being done there.
- src/server/data/automatedTestingData.js line 1349 inserts groups but does not do the equivalent of refreshGroupsDeepMeters. I manually did it and then the results were fine. This needs to be integrated into the code.
I wanted to ask if you want to work on any of the comments given our conversations. I'm happy for you to address the ones you want. It is also okay to indicate you would like them addressed by someone else. Please just let me/OED know.
| slope FLOAT, | ||
| intercept FLOAT, | ||
| PRIMARY KEY (source_id, destination_id) | ||
| source_id INTEGER REFERENCES units(id), |
There was a problem hiding this comment.
Fixed when resolving merge conflict.
| GROUP BY gdm.group_id, readings.start_timestamp, readings.end_timestamp | ||
| -- This ensures the data is sorted | ||
| ORDER BY readings.start_timestamp ASC; | ||
| IF (point_accuracy = 'daily'::reading_line_accuracy) THEN |
There was a problem hiding this comment.
Note on the point_accuracy calculated above (cannot put comment there):
Originally the code looped over the meters so all point accuracies had to be the same. Now it is getting it from the group views. Given this, I don't think they all need to be the same. This is what is done for meters above. I think the code (now or later) should be changed to do this.
There was a problem hiding this comment.
I'm going to leave this for someone else. I didn't do much with the group functions and I am not certain of how it needs to be modified.
| WHERE readings.graphic_unit_id = requested_graphic_unit_id | ||
| AND tsrange(start_stamp, end_stamp, '[]') @> readings.time_interval | ||
| -- This ensures the data is sorted | ||
| ORDER BY readings.time_interval ASC; |
There was a problem hiding this comment.
Here and in hourly, there is no grouping by group_id. I think it might interlace group ids without this if multiple ones are requested at the same time. This may be naturally fixed if address point accuracy per comment above.
| INNER JOIN unnest(group_ids) gids(id) ON dr.group_id = gids.id; | ||
|
|
||
| real_tsrange := tsrange(date_trunc_up('day', start_stamp), date_trunc('day', end_stamp)) * readings_max_tsrange; | ||
| -- Get the actual start/end time rounded to the nearest day from the range. |
There was a problem hiding this comment.
I think this comment might be better on the line above since that starts the rounding to the day. However, I just realized this relates to code done in another PR so it might get done there.
There was a problem hiding this comment.
Not adressing since it is more relevant to another PR.
|
@tdthompson650 I wanted to note that I just merged in @vyvyvyThao PR. After you finish any changes, I will take care of all the merge conflicts and verify that merging in that code into this PR does not cause any issues. |
- Tab indenting for sql. - Minor spacing. - Minor comment wording. - Reorder meter hour/daily view to be consistent.
…1546 This version starts up but has issues. It is mixing cik and cik_vary. It probably has other issues but wanted to commit at this point.
The path algorithm checks if segments exist and sometimes they do not if the conversion is backward so it is many or none (any).
cik_vary is currently copied to cik to get this temporary version working. The creation of the deep group views and the redoCikVary and the reading views is still done manually.
A number of files were messed up during the merge of timeVary. This attempts to fix them but testing, etc. is not yet working. Unfortunately, I could not figure out how to keep the original git attribution for most lines so they how indicate I did it. I want to note that most were originally done by @tfrendl and earlier commits show this.
- The reading views should now depend on cik_vary. - The conversion tests now properly create conversion and test more systematically.
- The insertion of cik from cik now does unique ones. - Test does redoCik so meter seen.
- This fixes a lot of tests & other code. - Suffix units & 3D tests still failing.
- It was trying to get slope/intercept from conversion but now in conversionSegment. The code only works if not time-varying so only one conversionSegment. This will need to be addressed. - Formatting.
- The edits to 3D were lost in the merge. Mostly this was the new version of 3D group but some other minor changes were also carried over.
Needed to add deep group refresh and do reading views after that. Also removed unneeded import.
- Remove slope/intercept from cik on client and server. - Note client still has slope/intercept in conversions because new code not yet merged. This means creating conversion does not work quite right and they cannot be deleted. - A few minor updates to files.
|
I wanted to leave a comment about the current status before I need to look at other OED work. The code is passing all the CI tests and a manual one I tried for one TV case. Here is a basic list of what is left at this point:
@tdthompson650 I made non-trivial changes due to the merge and other updates so please let me know if you have any thoughts. |
5 participants
Description
This pull request modifies the conversions so that they can vary with time. Materialized views were used to apply and store the converted readings to improve performance. The graphing functions were modified to retrieve the converted values from the views so that they no longer need to compute the conversion on-the-fly(except for raw reading conversions). The CIK table was modified to contain the start/end timestamp for the conversion, and the start_time was added to the composite primary key.
Partly Addresses #896
Type of change
Checklist
Limitations
Although conversions may vary with time, each conversion must be one hour in length and start/end on the hour.