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

Speed up planner plotting #121

Merged
merged 7 commits into from
Jul 6, 2023
Merged

Speed up planner plotting #121

merged 7 commits into from
Jul 6, 2023

Conversation

sea-bass
Copy link
Owner

@sea-bass sea-bass commented Jul 4, 2023

This PR significantly speeds up plotting using the technique described in https://stackoverflow.com/questions/53035858/plot-multiple-values-with-matplotlib-without-loop

To test, you can specifically try some of the multirobot demos / demos with PRMs in them.

@ibrahiminfinite
Copy link
Collaborator

There is a small bug from #119 which causes a "referenced before assignment" bug.
Since tgt_loc was replaced with goal_node. this line should also have that change.

I ran the ROS multirobot demo after adding that, there were no errors, but the rendering was spooky, with robots teleporting to different locations, see this video

@sea-bass
Copy link
Owner Author

sea-bass commented Jul 6, 2023

Nice catch on the bug! I pushed that fix directly to main.

The teleporting error is also happening on main right now... I suspect something happened with removing those extra variables in robot.py... will look into it

@sea-bass
Copy link
Owner Author

sea-bass commented Jul 6, 2023

Actually, this issue is because we're publishing commands before the GUI is started, and that causes things to go out of sync. I'll find a fix to wait til the GUI is running before sending this out.

@sea-bass
Copy link
Owner Author

sea-bass commented Jul 6, 2023

@ibrahiminfinite Try again? Just made some changes.

@ibrahiminfinite
Copy link
Collaborator

@ibrahiminfinite Try again? Just made some changes.

Ran it again, works smooth and the graph plotting is FAST.
Also very nice to see that the GUI is synced up.

Copy link
Collaborator

@ibrahiminfinite ibrahiminfinite left a comment

Choose a reason for hiding this comment

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

LGTM

@sea-bass sea-bass merged commit 850e00a into main Jul 6, 2023
4 checks passed
@sea-bass sea-bass deleted the planner-plot-speedup branch July 6, 2023 16:00
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