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 logging rates #146

Merged
merged 8 commits into from
May 16, 2023
Merged

Conversation

andermi
Copy link
Collaborator

@andermi andermi commented May 11, 2023

add sim-time-aware Rate class for controlling publishing in all controllers

@andermi andermi changed the base branch from main to andermi/launch_sim_pblog May 11, 2023 03:00
Signed-off-by: Michael Anderson <[email protected]>
Signed-off-by: Michael Anderson <[email protected]>
@andermi
Copy link
Collaborator Author

andermi commented May 11, 2023

@hamilton8415 using this branch, I can verify that the pub rates increase with RTF relative to wall, but stay 10Hz relative to sim clock

@hamilton8415
Copy link
Collaborator

Getting there! I did some runs with this and it's so nice to be able to run fast, really opens up what's possible...

It looks like perhaps the buoy AHRS (DataID = 3) has been overlooked? I plotted the time and time-between samples in the log files and everything lines up perfectly on 0.1seconds (when running faster than real-time), but the ^3 lines are still lagging. These are denoted "XB" in the plot below.

Capture

@hamilton8415
Copy link
Collaborator

And in another interesting plot, I change the pack-rate for the spring controller and the power converter using pbcmd while running at faster than realtime. For the most part the output behaves as can be seen in this plot.

Capture

The basic speed of the packets changes for sure, but one can see some issues with dither in the timesteps when the packet rate is at the highest speed (50Hz), it still averages a timestep of 0.02, but it's sometimes zero, and sometimes faster. Looking at the .csv files, there is sufficent resolution in the timestamps (3 digits).

@hamilton8415
Copy link
Collaborator

And finally, pbcmd is not exiting gracefully with these pack commands, they work fine though...

hamilton@WOLFPACK:~$ pc_PackRate 25
Executing pc_PackRate to Set the CANBUS packet rate from the Power Controller: 25 Hz
terminate called without an active exception
Aborted (core dumped)

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

Can you repeat your plots for running specific RTF's? Can you try physicsRTF: 2 and 3, 4, 5? wondering if there's a certain speed it breaks down.

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

I'm pretty certain it comes down to the physics step. If you'd like to have data available every 0.02 seconds, but our physics step is 0.1... We are probably going to have some zero-order-hold going on. I was letting gz-sim decide the timestamp when it updates data, and then the publisher would use that timestamp, but I could hide the ZOH and have the publisher set the stamp when it sends and we'd then have the correct dt in the logs...

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

I think that is the same issue with the AHRS. The simulated IMU sensor plugin is set to provide data at 50Hz to ensure precise data is available to the AHRS controller when it asks for it to publish at 10Hz which isn't possible with a timestep of 0.1

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

Can you repeat the test using a physics step of 0.02?

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

As for the pbcmd.... uhhggghghhhghghg

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

TC should be in the same boat as XB but I'm having trouble seeing those datapoints

@hamilton8415
Copy link
Collaborator

OK, I will check all of this. Does the RTF take an array of arguments and then run a number of runs?

@hamilton8415
Copy link
Collaborator

Yes, there is a chance that I ran the physics step slower than 50Hz for this test. Unfort I deleted the results but will re-run. makes sense...

I'm pretty certain it comes down to the physics step. If you'd like to have data available every 0.02 seconds, but our physics step is 0.1... We are probably going to have some zero-order-hold going on. I was letting gz-sim decide the timestamp when it updates data, and then the publisher would use that timestamp, but I could hide the ZOH and have the publisher set the stamp when it sends and we'd then have the correct dt in the logs...

_

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

Does the RTF take an array of arguments

No, that's one of the globals out of the matrix

@hamilton8415
Copy link
Collaborator

Here's an updated plot with the physics timestep at 0.01 and a variety of packet rates set via pbcmd. Still a bit of noise in there, but acceptable. Our typical matlab processing includes a step in which all of the controllers data points are interpolated onto a common time-base to facilitate analysis. And that worked fine...

Also, TC data seems fine, just the XB (^3) is not keeping up...

Capture

@hamilton8415
Copy link
Collaborator

Oh yes, sorry for the red-herring, my bad to ask it to give data more often than it was generating data...

@andermi
Copy link
Collaborator Author

andermi commented May 12, 2023

the dither there still isn't very satisfying... I'm guessing there's a race between the publish thread grabbing data and the simulation thread updating the data. I wasn't ever too happy with the low-/high-priority triple mutex thing. If it becomes a problem, I could look into a simpler/robust solution.

@andermi
Copy link
Collaborator Author

andermi commented May 16, 2023

@hamilton8415 can you run your plots for this commit? I think I may have fixed the AHRS pub rate

@hamilton8415
Copy link
Collaborator

Stand by, running now...

@hamilton8415
Copy link
Collaborator

hamilton8415 commented May 16, 2023

Seemingly unchanged for me, but we should probably do a little branch checking. I'm on fix_logging_rate here and add_pblog_args for _utils...

3, 1684202088.468, ,,,,,,,,,,,,,,,,,,,,,,,0.285, 0.637, 28.947, 0.002, 0.003, 0.070, -0.195, 0.109, 10.111, -0.108, 0.200, -0.108, 0.00000, 0.00000, 0.000, 0.000, ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
3, 1684202088.958, ,,,,,,,,,,,,,,,,,,,,,,,0.314, 0.362, 30.873, -0.001, -0.026, 0.071, -0.221, 0.196, 10.152, -0.278, 0.125, -0.278, 0.00000, 0.00000, 0.000, 0.000, ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
3, 1684202089.328, ,,,,,,,,,,,,,,,,,,,,,,,0.251, -0.532, 32.405, -0.005, -0.052, 0.070, -0.168, 0.228, 10.078, -0.396, 0.027, -0.396, 0.00000, 0.00000, 0.000, 0.000, ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
3, 1684202089.628, ,,,,,,,,,,,,,,,,,,,,,,,0.085, -1.508, 33.587, -0.012, -0.058, 0.067, -0.096, 0.208, 9.967, -0.462, -0.083, -0.462, 0.00000, 0.00000, 0.000, 0.000, ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
3, 1684202090.208, ,,,,,,,,,,,,,,,,,,,,,,,-0.580, -2.966, 35.654, -0.020, -0.023, 0.055, 0.122, 0.082, 9.668, -0.475, -0.333, -0.475, 0.00000, 0.00000, 0.000, 0.000, ,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,

Also, long buffer times when running RTF>1 present, here on the local machine too...

@andermi
Copy link
Collaborator Author

andermi commented May 16, 2023

you should be on @rhenthorn 's branch for _utils

@andermi
Copy link
Collaborator Author

andermi commented May 16, 2023

uhhh oops. I definitely found the problem this time. I never enabled the use_sim_time parameter for the AHRS...

Signed-off-by: Michael Anderson <[email protected]>
@andermi
Copy link
Collaborator Author

andermi commented May 16, 2023

the fix is in!

@hamilton8415
Copy link
Collaborator

Indeed, looking good.

@andermi andermi merged commit 6091a28 into andermi/launch_sim_pblog May 16, 2023
@andermi andermi deleted the andermi/fix_logging_rates branch May 16, 2023 02:07
andermi added a commit that referenced this pull request May 29, 2023
* add support for pblog

Signed-off-by: Michael Anderson <[email protected]>

* change default pblog root; add latest_rosbag symlink

Signed-off-by: Michael Anderson <[email protected]>

* Fixed issue by which wave-random seed wasn't being passed to Incident
wave generator

* Fix logging rates (#146)

* add sim-time-aware Rate to controllers for publishing

Signed-off-by: Michael Anderson <[email protected]>

* forgot shutdowns to stop hanging

Signed-off-by: Michael Anderson <[email protected]>

* add copyright

Signed-off-by: Michael Anderson <[email protected]>

* linters

Signed-off-by: Michael Anderson <[email protected]>

* add async proxy to await future for pack rate calls (#147)

Signed-off-by: Michael Anderson <[email protected]>

* use sim time from postupdate instead of imu time

Signed-off-by: Michael Anderson <[email protected]>

* enable use_sim_time for AHRS

Signed-off-by: Michael Anderson <[email protected]>

---------

Signed-off-by: Michael Anderson <[email protected]>

* linters

Signed-off-by: Michael Anderson <[email protected]>

* moved tf sensors to heavecone link; rotated imu to match physical

Signed-off-by: Michael Anderson <[email protected]>

* add gps

Signed-off-by: Michael Anderson <[email protected]>

---------

Signed-off-by: Michael Anderson <[email protected]>
Co-authored-by: Andrew Hamilton <[email protected]>
@andermi andermi restored the andermi/fix_logging_rates branch September 19, 2023 16:21
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.

2 participants