Skip to content

Add path shortening utils and consolidate examples#33

Merged
sea-bass merged 24 commits into
mainfrom
path-shortening
Jul 28, 2025
Merged

Add path shortening utils and consolidate examples#33
sea-bass merged 24 commits into
mainfrom
path-shortening

Conversation

@eholum

@eholum eholum commented Jul 21, 2025

Copy link
Copy Markdown
Collaborator

Resolves #19.

Shamelessly converting https://github.com/sea-bass/pyroboplan/blob/main/src/pyroboplan/planning/path_shortcutting.py.

The quality of the example is highly dependent on the original path, but I suppose that's to be expected. Here's a good run:

image

@eholum eholum self-assigned this Jul 21, 2025
@eholum eholum changed the title Add path shortening utils [WIP] Add path shortening utils Jul 21, 2025
@eholum
eholum force-pushed the path-shortening branch from 5460752 to c0932ed Compare July 21, 2025 15:29

@sea-bass sea-bass left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you know me, reviewing the draft PRs like a sociopath

Comment thread roboplan/src/core/path_utils.cpp Outdated
Comment thread roboplan/src/core/path_utils.cpp Outdated
Comment thread roboplan/src/core/path_utils.cpp Outdated
Comment thread roboplan/src/core/path_utils.cpp Outdated
Comment thread roboplan/src/core/path_utils.cpp Outdated
Comment thread roboplan/src/core/path_utils.cpp Outdated
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
@eholum

eholum commented Jul 22, 2025

Copy link
Copy Markdown
Collaborator Author

The job was not started because recent account payments have failed or your spending limit needs to be increased. Please check the 'Billing & plans' section in your settings

Uh oh.

@eholum
eholum force-pushed the path-shortening branch from 4a586ad to 2d07f7a Compare July 22, 2025 12:35
@sea-bass

Copy link
Copy Markdown
Collaborator

The job was not started because recent account payments have failed or your spending limit needs to be increased. Please check the 'Billing & plans' section in your settings

Uh oh.

Gahh... so I guess I either need to pay up or we make this repo public. Preferences?

@eholum

eholum commented Jul 22, 2025

Copy link
Copy Markdown
Collaborator Author

Gahh... so I guess I either need to pay up or we make this repo public. Preferences?

Third option, move it to an org? Would that reset the count?

@eholum eholum mentioned this pull request Jul 22, 2025
@sea-bass

Copy link
Copy Markdown
Collaborator

Third option, move it to an org? Would that reset the count?

Yeah I think that's what I meant by making it public. The org itself cannot be private, and I'm additionally not sure right now if one can have a private repo in a free tier org. I can check later.

@sea-bass

Copy link
Copy Markdown
Collaborator

Screenshot_20250722_090456_Chrome

@eholum

eholum commented Jul 22, 2025

Copy link
Copy Markdown
Collaborator Author

4th option: Give me permissions and I'll add this to my linux runner under my desk.

@sea-bass

Copy link
Copy Markdown
Collaborator

Your CI is back, kind sir.

@eholum

eholum commented Jul 22, 2025

Copy link
Copy Markdown
Collaborator Author

Your CI is back, kind sir.

Thank goodness I can see that broken rolling build, again.

@eholum eholum left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

checking things

Comment thread bindings/src/roboplan/viser_visualizer.py Outdated
Comment thread bindings/src/roboplan/viser_visualizer.py
Comment thread bindings/src/roboplan_ext.cpp
@eholum eholum changed the title [WIP] Add path shortening utils Add path shortening utils Jul 25, 2025
@eholum

eholum commented Jul 25, 2025

Copy link
Copy Markdown
Collaborator Author

Ah gonna stop playing with this since I think this is a reasonable functional baseline.

@eholum
eholum marked this pull request as ready for review July 25, 2025 14:17

@sea-bass sea-bass left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Cool! Just a few comments

from roboplan.viser_visualizer import ViserVisualizer


def visualizePath(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So once the Franka PR lands, I think this is what we'll need:

  • Factor out the existing visualizePath to two separate functions, visualizeRRT and visualizePath, which accepts the name of the thing to visualize as an input arg.
  • The original example will call one of each
  • This example will call visualizeRRT once, and visualizePath twice (for the raw and shortcutted paths)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Follow-up: Maybe we instead just want to modify the existing RRT examples and add a flag for whether we want shortcutting or not?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done and done.

I'm assuming it's preferable just to have all of the configurable parameters directly in code rather than adding arguments or something to these example scripts to do more code consolidation?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Either/or. Viser uses this other package named tyro which makes command line args pretty nice, if you wanna check it out. The viser examples are a good reference.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh that is need. Do you want to tackle that here? I would probably merge the ik and rrt examples together and make everything an argument, including the robot models. Thoughts?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That sounds great -- up to you if you wanna split it off or club it all together.

Comment thread bindings/src/roboplan/viser_visualizer.py
Comment thread roboplan/include/roboplan/core/path_utils.hpp Outdated
@eholum

eholum commented Jul 27, 2025

Copy link
Copy Markdown
Collaborator Author

We may want to land this straight to Pinocchio then...

Working on it but my little machine is on the struggle bus right now.

@sea-bass

Copy link
Copy Markdown
Collaborator

Working on it but my little machine is on the struggle bus right now.

I'll let you in on a secret. I never actually built Pinocchio for my visualizer PRs. I just added the files to my install space and copied them into the repo when they worked.

@eholum

eholum commented Jul 28, 2025

Copy link
Copy Markdown
Collaborator Author

I never actually built Pinocchio for my visualizer PRs

Rule number 1: I never half-ass anything. I'm a whole-ass man.

@eholum eholum changed the title Add path shortening utils Add path shortening utils and consolidate examples Jul 28, 2025
@eholum

eholum commented Jul 28, 2025

Copy link
Copy Markdown
Collaborator Author

Okay added tyro. It's very handy because now one can do things like,

python3 examples/example_rrt.py --host ubuntu-linux.shared --model ur5 --include_shortcutting --max_connection_distance 0.5 --max_planning_time 10 --max_nodes 100000 --goal_biasing_probability 0.001 --rrt_connect
image

And the autogenerated help is quite pretty,

$ python3 examples/example_ik.py --help
usage: example_ik.py [-h] [OPTIONS]

Run the IK example with the provided parameters.

╭─ options ──────────────────────────────────────────────────────────────────────────────╮
│ -h, --help              show this help message and exit                                │
│ --model STR             The name of the model to user (ur5 or franka). (default: ur5)  │
│ --max-iters INT         Maximum number of iterations for the IK solver. (default: 100) │
│ --step-size FLOAT       Integration step size for the IK solver. (default: 0.25)       │
│ --goal-biasing-probability FLOAT                                                       │
│                         (default: 0.15)                                                │
│ --max-nodes INT         (default: 1000)                                                │
│ --max-planning-time FLOAT                                                              │
│                         (default: 3.0)                                                 │
│ --rrt-connect, --no-rrt-connect                                                        │
│                         (default: False)                                               │
│ --include-shortcutting, --no-include-shortcutting                                      │
│                         (default: False)                                               │
│ --host STR              The host for the ViserVisualizer. (default: localhost)         │
│ --port STR              The port for the ViserVisualizer. (default: 8000)              │

@sea-bass

Copy link
Copy Markdown
Collaborator

That's a lovely ball of yarn.

@eholum

eholum commented Jul 28, 2025

Copy link
Copy Markdown
Collaborator Author

Sigh. Does anyone like ros? (I do)

@sea-bass

sea-bass commented Jul 28, 2025

Copy link
Copy Markdown
Collaborator

Sigh. Does anyone like ros? (I do)

I ran into this pain too. Just pin the typing_extensions version in your install command:

&& pip3 install "numpy<2.0.0" typing_extensions==4.10.0 viser opencv-contrib-python-headless \

Or you can pin tyro itself. See brentyi/tyro#324

Comment thread .docker/ros/Dockerfile Outdated

@sea-bass sea-bass left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Amazing!

@sea-bass
sea-bass merged commit 87c6375 into main Jul 28, 2025
8 of 9 checks passed
@sea-bass
sea-bass deleted the path-shortening branch July 28, 2025 23:33
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.

Add path shortcutting algorithm

2 participants