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

Repurpose the "Sim/Real Time" field on the sim control panel #1626 #1627

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

alexlin0
Copy link
Contributor

Added calculations in the realtime sync monitor job to calculate the average sim/realtime ratio for the past 100 frames. This is saved to a new variable, actual_run_ratio. Changed the sim control panel to display the actual_run_ratio value.

Added calculations in the realtime sync monitor job to calculate the
average sim/realtime ratio for the past 100 frames.  This is saved to
a new variable, actual_run_ratio.  Changed the sim control panel to
display the actual_run_ratio value.
how does this compile locally but not in CI?  Didn't include cmath.
@coveralls
Copy link

Coverage Status

coverage: 55.894% (+0.08%) from 55.818%
when pulling 4b34003 on 1626-sim-real-time-field
into dafddff on master.

@hchen99
Copy link
Contributor

hchen99 commented Dec 20, 2023

Thanks, Alex! We're saving this for the first thing after new year.

@hchen99
Copy link
Contributor

hchen99 commented Jan 15, 2024

Just to verify:

  • This pull request repurposed Sim / Real Time field of Sim Control gui to show the average ratio saved as actual_run_ratio for past 100 frames.
  • The user still uses the existing Throttle feature to change rt_clock_ratio.
  • The actual average ratio is calculated based on rt_clock_ratio.

What do you think if changing to only show rt_clock_ratio in Sim / Real Time Field especially for reflecting what is changed by user using Throttle when sim is not started?

@alexlin0
Copy link
Contributor Author

alexlin0 commented Jan 18, 2024

I believe that nobody uses rt_clock_ratio, sims are either run at 1.0x realtime or they are run non-realtime as fast as possible. It is a feature that could be deleted and no one would miss it.

I assume we want to keep the feature. In this case, I would not suggest using the field for two different values. If we want to show the actual sim/real ratio and the rt_clock_ratio set by the user, then we should have 2 fields on the sim control panel. I did think about this, but I was lazy to add another field and determined using the unused field was easier.

@hchen99
Copy link
Contributor

hchen99 commented Jan 19, 2024

I believe that nobody uses rt_clock_ratio, sims are either run at 1.0x realtime or they are run non-realtime as fast as possible. It is a feature that could be deleted and no one would miss it.

I assume we want to keep the feature. In this case, I would not suggest using the field for two different values. If we want to show the actual sim/real ratio and the rt_clock_ratio set by the user, then we should have 2 fields on the sim control panel. I did think about this, but I was lazy to add another field and determined using the unused field was easier.

Thanks for your inputs. Ok, made sense now. If that's the case, probably no need to add another field. Instead, probably we can update the Throttle dialog to keep what was selected previously so user has a place to check if they know where to change the ratio once this PR is merged.

@sharmeye sharmeye merged commit 7547d36 into master Jan 23, 2024
11 checks passed
@sharmeye sharmeye deleted the 1626-sim-real-time-field branch January 23, 2024 16:47
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