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

Add replay button to sensor and asset pages #463

Merged
merged 39 commits into from Sep 8, 2022
Merged

Add replay button to sensor and asset pages #463

merged 39 commits into from Sep 8, 2022

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Jul 15, 2022

Closes #436

  • Add replay button to sensor page
  • Add replay button to asset page
  • Get chart data including older beliefs
  • Loop through data replaying the past
  • Show simulation time underneath replay button
  • Speed up chart responsiveness: draw only the most recent beliefs in the chart
  • Speed up slicing: pop and append data
  • Speed up chart updating: remove only obsolete beliefs and insert only new beliefs [did not result in faster chart updating]

Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x added the UI label Jul 15, 2022
@Flix6x Flix6x self-assigned this Jul 15, 2022
@coveralls
Copy link
Collaborator

coveralls commented Jul 15, 2022

Pull Request Test Coverage Report for Build 3015290146

  • 5 of 13 (38.46%) changed or added relevant lines in 3 files are covered.
  • 31 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.04%) to 65.056%

Changes Missing Coverage Covered Lines Changed/Added Lines %
flexmeasures/data/models/time_series.py 3 6 50.0%
flexmeasures/data/models/generic_assets.py 0 5 0.0%
Files with Coverage Reduction New Missed Lines %
flexmeasures/data/models/time_series.py 1 70.03%
flexmeasures/data/schemas/generic_assets.py 9 83.05%
flexmeasures/data/queries/utils.py 21 51.32%
Totals Coverage Status
Change from base Build 2999633885: -0.04%
Covered Lines: 6477
Relevant Lines: 9349

💛 - Coveralls

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@nhoening
Copy link
Contributor

I feel a pause button (next to Play) would be very useful to have!

…w slice to simulatedData instead of reslicing result all over

(also time function performance)

Signed-off-by: F.N. Claessen <felix@seita.nl>
@nhoening nhoening changed the title Add replay button to sensor page Add replay button to sensor and asset pages Jul 21, 2022
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 21, 2022

I feel a pause button (next to Play) would be very useful to have!

Did you notice the play button already animates into a pause button?

@nhoening
Copy link
Contributor

No, I did not!

@Flix6x
Copy link
Contributor Author

Flix6x commented Jul 21, 2022

Please note that the current code includes some helpful logging statements in your browser console: which functionality is being called when hitting the button, and some statements about execution time. I'll remove those timers at a later point.

When selecting a longer time periode (more than a week), the simulation slows down a bit. This seems to be mainly from reloading the data into the graph. Slicing the data takes 10-30 ms, and loading that data into the graph takes 30-200 ms, depending on how much data is shown in the graph. I tried different approaches here, but fell back to the original approach of telling vega to remove all previous data and to insert a completely new dataset at each iteration. Updating the view with removing only a few data points (representing obsolete beliefs) and inserting only a few data points (representing the most recent new beliefs) actually made it slower.

Flix6x and others added 16 commits August 8, 2022 20:25
…ts dataset

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Instead of having multiple columns with event_values, named by sensor_id, we switched to one column containing the sensor_id, in addition to the column holding the event_values.

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
…taFrame and refactor: move updateBeliefs function into replay-utils.js

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
…ained in leftsidepanel block)

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x added this to the 0.12.0 milestone Aug 12, 2022
@Flix6x Flix6x marked this pull request as ready for review August 12, 2022 11:28
@Flix6x Flix6x requested a review from nhoening August 12, 2022 11:29
@nhoening nhoening added this to In progress in Update UI views for Sensors and Assets via automation Aug 24, 2022
@nhoening nhoening moved this from In progress to Review in progress in Update UI views for Sensors and Assets Aug 24, 2022
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Great work.

I concentrated on the JavaScript and have some ideas to improve readability.

But it does seem to work well, I tried it out locally.

flexmeasures/ui/templates/base.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/base.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/base.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/base.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/base.html Outdated Show resolved Hide resolved
flexmeasures/ui/static/js/replay-utils.js Outdated Show resolved Hide resolved
flexmeasures/ui/static/js/replay-utils.js Outdated Show resolved Hide resolved
flexmeasures/ui/templates/base.html Outdated Show resolved Hide resolved
flexmeasures/ui/static/js/replay-utils.js Show resolved Hide resolved
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x requested a review from nhoening September 5, 2022 12:45
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

I like it.
Just a few ideas to increase readability even more.

flexmeasures/ui/static/js/replay-utils.js Outdated Show resolved Hide resolved
flexmeasures/ui/templates/base.html Outdated Show resolved Hide resolved
flexmeasures/ui/templates/base.html Show resolved Hide resolved
flexmeasures/ui/templates/base.html Show resolved Hide resolved
Update UI views for Sensors and Assets automation moved this from Review in progress to Reviewer approved Sep 6, 2022
@nhoening
Copy link
Contributor

nhoening commented Sep 6, 2022

One more comment: I notice that a few lines in base.html do not have a semicolon:

220, 251, 259-261, 263, 265, 267-272, 308, 309, 311-319, etc.

I believe we should add them, or otherwise follow one formatting style.

Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x merged commit d491371 into main Sep 8, 2022
@Flix6x Flix6x deleted the replay branch September 8, 2022 13:12
Update UI views for Sensors and Assets automation moved this from Reviewer approved to Done Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Add option to replay what happened
3 participants