lerobot icon indicating copy to clipboard operation
lerobot copied to clipboard

feat(policy): Add DexVLA policy [Community Contributed]

Open JayceWen opened this issue 9 months ago • 12 comments

What this does

Add new VLA policy: DexVLA.

JayceWen avatar Feb 24 '25 11:02 JayceWen

@imstevenpmwork Hi, is this "Review required At least 1 approving review is required by reviewers with write access." means you need review my code or someone else?

JayceWen avatar Mar 11 '25 07:03 JayceWen

Hello @JayceWen ! Thanks for your contribution. Indeed, someone from the team needs to review this PR before it can get merged to upstream. I will ping the policy experts 😄

imstevenpmwork avatar Mar 14 '25 16:03 imstevenpmwork

Hi @JayceWen,

After discussing with the team, we've decided that we'd like to merge this PR. But before doing so, we'd like you to provide a bit more context about it. Specifically, it would be helpful to include things like a README for the policy, an example, or some supporting visuals (e.g., videos, images, or graphs), maybe also an explanation on how you tested and which set-up you used.

Since we haven’t had many PRs introducing new policies yet, we're still shaping the guidelines for contributors. Your PR could serve as a great reference point 😊

Thanks for your work!

imstevenpmwork avatar Mar 14 '25 21:03 imstevenpmwork

Hi @imstevenpmwork,

Thanks for replying and it's very exciting to contribute to such an open-sourced project designed exactly for robotics. I have added more commits to improve the code and a README file for quickly train the policy.

Thanks for your help.

JayceWen avatar Mar 18 '25 07:03 JayceWen

Hello @JayceWen , happy to see this taking shape 😄 some comments:

  • Can you add some details about training time, inference speed, and what data it has been pretrained on ?
  • Can you mention explicitly at the top of the readme that this policy is "Community Contributed" ?

Once you finish polishing the rough edges, I will ask the policy guys to review the code and merge this

imstevenpmwork avatar Mar 18 '25 10:03 imstevenpmwork

Hi, @imstevenpmwork , I have already added descriptions on training time, inference speed, and pretraining data. Besides, I have added "Community Contributed" at the beginning of the README.

JayceWen avatar Mar 19 '25 06:03 JayceWen

Hi @imstevenpmwork,

I have checked the requirements for dexvla and corrected the required version of qwen_vl_utils.

JayceWen avatar Mar 20 '25 05:03 JayceWen

Hi @JayceWen

Thanks for putting in the effort on this—great work so far! I just have a few follow-up questions and requests to ensure everything aligns with the project’s standards:

  1. License Headers: Could you please add the proper license headers to all the newly added files?

  2. Module Structure: I noticed there are two separate modules (policy_heads and qwe2_vla). Was there a specific reason for splitting them into subfolders? For consistency, I’d suggest keeping all the register() calls in configuration_dexvla.py and placing the scripts directly under dexvla/. That said, I’m open to hearing your thoughts—let me know what you think! 😊

  3. Commit History: Since this is a big PR, I’d prefer to Rebase & Merge it. To make the commit history cleaner and more meaningful, could you please rebase/squash/rename the commits as you see fit?

Let me know if you have any questions or need help with any of this!

imstevenpmwork avatar Mar 25 '25 13:03 imstevenpmwork

Thanks for your contributions @JayceWen

I noticed there are some comments in Chinese in the code. To maintain consistency across the codebase, we prefer keeping comments in English only.

Would you mind updating these to English when you have a chance? We really appreciate your help in keeping the codebase clean and accessible for everyone!

imstevenpmwork avatar Apr 01 '25 08:04 imstevenpmwork

Hi @imstevenpmwork,

Thanks for the feedback.

  1. I have already added the license header and cleared the commit history.

  2. Regarding the model structure, since DexVLA combines a VLM backbone and a policy head, I suggest separating these two components to help users better understand our framework. Additionally, this separation makes it easier to replace DexVLA with a new policy head or VLM backbone. What's more, current VLAs tend to follow a dual-system architecture. Maybe a hierarchical structure would better organize the code?

  3. Thanks for reminder, I will change them.

Thank you again for your support, I will continue working on this PR.

JayceWen avatar Apr 02 '25 12:04 JayceWen

Hi @imstevenpmwork,

I noticed that the process has been stuck at the "Review required" stage for a long time. I want to check if there are any questions or concerns regarding the code, or if there's anything further I can assist with to help move things forward. I'm more than happy to continue working on improving the code as needed.

Please feel free to let me know if there is anything that requires further attention or improvement.

I really appreciate for your support and look forward to your reply. 😄

JayceWen avatar Apr 16 '25 08:04 JayceWen

Hi @danaaubakirova, I’ve noticed that the code review has been ongoing for over a month. Is there any issue or anything else that needs attention? Please feel free to reach out, and I’d be happy to assist in resolving any problems.

JayceWen avatar May 12 '25 03:05 JayceWen