-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat(scripts): Introduce build_inference_frame/make_robot_action util to easily allow API-based Inference
#2143
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
feat(scripts): Introduce build_inference_frame/make_robot_action util to easily allow API-based Inference
#2143
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the preprocessing steps for policy inference by introducing a new build_inference_frame function that encapsulates the conversion of robot observations into the format expected by machine learning models. This addresses issue #2142 by providing a clean interface between robot observations and model inference.
Key changes:
- Added
build_inference_framefunction to centralize observation preprocessing logic - Replaced inline preprocessing code in
predict_actionwith a call to the new function - The new function handles tensor conversion, image normalization, device transfer, and metadata addition
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/lerobot/policies/utils.py | Adds the new build_inference_frame function with observation preprocessing logic |
| src/lerobot/utils/control_utils.py | Replaces inline preprocessing with call to build_inference_frame function |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the linked issue and this PR's goal. However, instead of just refactoring into a function, I suggest a more robust solution: implement the linked code as a formal processor step within the policy's preprocessor.
This would make the main control loop much cleaner:
obs = robot.get_observation()
obs = preprocess(obs) # Now includes the new step
action = model.select_action(obs)
action = postprocess(action)
# robot.send_action(action)This approach also addresses the root cause mentioned in point # 7 of the previous review: the flawed assumptions in batch_to_transition. By moving this logic to a processor, we make progress on point #1 of that same comment, which is to migrate policy boilerplate into proper processor steps.
Shall we proceed with this processor-based approach, or would you prefer to stick with the current PR's function-based refactor? (I'm good either way)
Thanks anyways for taking a look into this !
build_inference_frame util to easily allow API-based Inference
I would stick to the current PR and open a "good first issue" for the community to implement this as part of the pipeline system. The main rationale for this choice is that it is the solution which would allow us to ship the tutorial the earliest (i.e., tomorrow) without delaying release further. Also, I think there is a rather big CI blocker for the pipeline system: the need to be constantly updating all the models on the hub whenever we change the pipeline used, which is why I'd argue we should choose a bit more carefully when to roll out changes to the pipelines already uploaded to avoid disruptions. For instance, I think updates to the pipelines used by the model (such as the one you're describing here) could be introduced within a future 0.x.0 release. Happy to discuss this further tho! |
|
@imstevenpmwork just flagging that 06654e1 adds a very similar modification on the policy's outputs too I added this modification to this PR just to move fast. They are conceptually deriving from the same problem |
build_inference_frame util to easily allow API-based Inferencebuild_inference_frame/make_robot_action util to easily allow API-based Inference
…on to only perform data type handling (whole conversion is: keys matching + data type conversion)
|
Hey @imstevenpmwork 👋 I wanted to let you know (1) I succesfully tested these last changes using the CLI As per (2), I think I unnecessarily called
The new small function |
|
After merging the slight changes from #2198 |
imstevenpmwork
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API examples will be added later. Consider moving these functions to processor steps in the future.
Thanks !
…util to easily allow API-based Inference (huggingface#2143) * fix: expose a function explicitly building a frame for inference * fix: first make dataset frame, then make ready for inference * fix: reducing reliance on lerobot record for policy's ouptuts too * fix: encapsulating squeezing out + device handling from predict action * fix: remove duplicated call to build_inference_frame and add a function to only perform data type handling (whole conversion is: keys matching + data type conversion) * fix(policies): right utils signature + docstrings (huggingface#2198) --------- Co-authored-by: Steven Palma <[email protected]>
…util to easily allow API-based Inference (#2143) * fix: expose a function explicitly building a frame for inference * fix: first make dataset frame, then make ready for inference * fix: reducing reliance on lerobot record for policy's ouptuts too * fix: encapsulating squeezing out + device handling from predict action * fix: remove duplicated call to build_inference_frame and add a function to only perform data type handling (whole conversion is: keys matching + data type conversion) * fix(policies): right utils signature + docstrings (#2198) --------- Co-authored-by: Steven Palma <[email protected]>
What this does
Solves #2142, introducing an encapsulator to interface robot observations and models
build_inference_frame.Crucially, this PR enables a block of code like the following to run without any issues: