Closed HiroIshida closed 3 weeks ago
@alexander-soare Could you review this PR? :)
@alexander-soare Thank you for your review! I fixed the name and applied your suggestions. also, modify the test code and checked that it works https://gist.github.com/HiroIshida/5d0abbec0fdd0d140c5262eb1301c98e
@HiroIshida Thanks a lot for this PR! The quality check Is not passing. Otherwise, LGTM. I will approve after the issue is fixed:)
@danaaubakirova Thanks. I applied the formatter.
What this does
This PR enables to configure multiple rgb encoders for each cameras, which is done in the original diffusion policy paper. Not for sure in general setting, but at least in my task, this seems to increase the success rate of deployment slightly. Current diffusion policy shared the single RGB encoder for all cameras. This fixes https://github.com/huggingface/lerobot/issues/483
How it was tested
a) regression test
Prepare the following script and check that the behavior of the policy does not change before and the after the PR by comparing the hash value of the output.
Before this PR
After this PR
You can see the match of hash values.
b) validity check
The above test include the case (see case3 ), which is run only when the
DiffusionConfig
hasrgb_encoder_per_camera
attribute, so activate only after this PR. And as you can see, there is no error occured and output the action.c) tested on real robot
I trained the diffusion policy with the
rgb_encoder_per_camera=True
, and checked that policy worked well. I cannot say concrete stuff but the performance is slightly better than the original.How to checkout & try? (for the reviewer)
Try the above script and please check the hash value before and after this PR. Before the run, you need to save the following yaml file as
'stats-diffusion.yaml
.