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

Fix envision button skip to newest when resumed #663

Merged
merged 10 commits into from
Mar 15, 2021

Conversation

JingfeiPeng
Copy link
Contributor

Currently there is a problem with envision's pause button, pausing works, but when resumed, envision will skip directly to the most latest timestamp. This PR fixes this issue.

Summary of changes:
1: in client.js, frames are obtained from the end of the list, which has the most current frame. Changed this to obtain frames from beginning of list.
2: Added a sleep function to have a minimum amount of wait time before loading the next frame. Since after done step 1, when paused, there will be a long list of existing frames waiting to be fetched, if we simply fetch them from list and update worldstate, the update is too fast for react to cause re-render. Previously this wasn't a problem since the speed of giving out available frames is 1 frame per eposide on the smarts side. Envision will always get the frame when it is available and wait for the next available frame.

@@ -79,7 +79,7 @@ function App({ client }) {
// checks if there is new simulation running every 3 seconds.
const interval = setInterval(fetchRunningSim, 3000);
return () => clearInterval(interval);
}, []);
}, [routeMatch]);
Copy link
Contributor Author

@JingfeiPeng JingfeiPeng Mar 9, 2021

Choose a reason for hiding this comment

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

This 1 line fixes a different bug/warning. the history was trying to push the existing route on line 73 because the value of matchedSimulationId could be stale. This results in a warning when running envision in dev mode. Adding routeMatch here makes matchedSimulationId have the most updated value

Copy link
Collaborator

@Gamenot Gamenot left a comment

Choose a reason for hiding this comment

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

A question.

onElapsedTimesChanged(...elapsed_times);

// play the frames every 50ms
await sleep(50);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please point me to where the rate that Smarts is sending is controlled? (I saw your comment that it's also 20fps, but I'm just curious to look at it.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, SMARTS is sending frame to envision at the following line: https://github.com/huawei-noah/SMARTS/blob/master/smarts/core/smarts.py#L970 self._envision.send(state)

Here smarts is just sending the frame whenever an episode is done. 20fps is just an estimate. Since when I use 20fps, the visulation looks normal when transitioning from before pause -> pause -> after resume. If I use a higher fps ( sleep less than 50ms) then i can see the vehicle looks like it is driving way faster after resume for a period of time. Then slows down again when reached the latest frame

@@ -65,7 +65,7 @@ const treeData = [
],
},
{
title: "Inclucdes Social Agents",
title: "Includes Social Agents",
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤣

Comment on lines +178 to +181
await sleep(
msInSec * (currentTime - prevElapsedTime) -
(Date.now() - waitStartTime)
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this biases towards losing time although it means we will never skip frames.

// we deduct this amount from the time we will be waiting
await sleep(
msInSec * (currentTime - prevElapsedTime) -
(Date.now() - waitStartTime)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible for this to end up being negative? If so, should we avoid that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in very rare situations where the waiting time to get the next frame exceeds the time difference between the frames the sleep time could be negative. However this is fine, calling sleep with a negative numebr sleep(-1000) would return instantly just like sleep(0)

@JingfeiPeng JingfeiPeng merged commit 6f56b0f into develop Mar 15, 2021
@JingfeiPeng JingfeiPeng deleted the fix_envision_button_skip_to_newest_when_resumed branch April 13, 2021 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants