Skip to content

Conversation

aliberts
Copy link
Collaborator

Note: I've split #54 into 2 PRs since they cover mainly unrelated changes

This adds:

  • test_aloha in test_envs.py which was missing
  • Code coverage
  • More end-to-end tests:
    • train ACT on ALOHA (no online, offline_steps=2)
    • eval ACT on ALOHA (from train checkpoint)
    • eval ACT on ALOHA (no checkpoint, policy is None)
    • train Diffusion on PushT (no online, offline_steps=2)
    • eval Diffusion on PushT (from train checkpoint)
    • eval Diffusion on PushT (no checkpoint, policy is None)
    • train TDMPC on Simxarm (with balanced offline/online_steps=1
    • eval TDMPC on Simxarm (from train checkpoint)
    • eval TDMPC on Simxarm (no checkpoint, policy is None)

TODO

@aliberts aliberts requested a review from Cadene March 27, 2024 12:32
pre-commit = "^3.6.2"
debugpy = "^1.8.1"
pytest = "^8.1.0"
pytest-cov = "^5.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we have something in README in ### Contribute section indicating how to install dev dependencies? Are they installed when we do poetry install the first time?

Copy link
Collaborator Author

@aliberts aliberts Mar 27, 2024

Choose a reason for hiding this comment

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

They are installed by default with poetry install. They can be skipped with poetry install --without dev.
Group dependencies are a conveniency offered by poetry for several purposes (one of which is staged build). In our case we just use it to differentiate those dev dependencies from the core ones (but it's really just labeling here)

("sim_transfer_cube", True, True),
],
)
def test_aloha(task, from_pixels, pixels_only):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

Copy link
Collaborator

@Cadene Cadene left a comment

Choose a reason for hiding this comment

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

The CI took 12mn to run. It's too much. What could we do to parallelize? Not mandatory, but it would be nice to address this in this PR.

@aliberts
Copy link
Collaborator Author

The CI took 12mn to run. It's too much. What could we do to parallelize? Not mandatory, but it would be nice to address this in this PR.

Once merged, dependencies will be cached so it should take ~4mns less.
Here are my thoughts on how to cut down time further:

  • cache libegl1-mesa-dev install (~30s less)
  • remove the ACT train on Aloha with no policy test, or at least reconfigure it (takes ~2mn right now, it shouldn't be that long)
  • More generally change parameters for all end-to-end tests (I'm thinking about the number of steps, horizon, num_slices, batch_size...)

Do you see any other params that could impact run time significantly?

I will try those ideas before merging to see what works best.

@aliberts aliberts force-pushed the user/aliberts/2024_03_27_improve_ci branch from a253c45 to 404b8f8 Compare March 28, 2024 09:36
@aliberts
Copy link
Collaborator Author

I'm merging this with the eval ACT on Aloha (policy is None) test deactivated so that we can move on for now.
I will take care of this in a further PR.

@aliberts aliberts merged commit 2a98cc7 into main Mar 28, 2024
@aliberts aliberts deleted the user/aliberts/2024_03_27_improve_ci branch April 27, 2024 08:16
menhguin pushed a commit to menhguin/lerobot that referenced this pull request Feb 9, 2025
…_03_27_improve_ci

Add code coverage, more end-to-end tests
Kalcy-U referenced this pull request in Kalcy-U/lerobot May 13, 2025
…ove_ci

Add code coverage, more end-to-end tests
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