search

huggingface / lerobot

🤗 LeRobot: Making AI for Robotics more accessible with end-to-end learning
Apache License 2.0
6.15k stars 529 forks source link

[Improvement] Best way to set MUJOCO_GL #55

Open aliberts opened 5 months ago

aliberts commented 5 months ago

Right now, some online training won't work when the env uses mujoco if the environment variable MUJOCO_GL has not been set to egl.

Since there's no indication for that in the current README, I'm creating this issue to discuss the best way to address this moving forward. Here are the 2 ways that I see for how we could solve this:

1. Explicitly tell the user in the README to set it:

export MUJOCO_GL=egl

2. Set it directly in the code (and make it invisible to the user):

os.environ["MUJOCO_GL"] = "egl"

I don't really know what's the best way here. What do you think @Cadene @alexander-soare?

alexander-soare commented 5 months ago

IMO we should avoid making the user export env variables at almost all costs (it's cumbersome, and if you have a policy of allowing it, there will be more).

Questions:

  1. Is it potentially intrusive or damaging to set it silently? Would the user really want to know about it?
  2. Can we check under what conditions it needs to be set and only set it under those (Gemini tells me it's needed when running serverless, and I bet there's a way to detect that).
aliberts commented 5 months ago

IMO we should avoid making the user export env variables at almost all costs (it's cumbersome, and if you have a policy of allowing it, there will be more).

I agree, it's probably not good practice

  1. Is it potentially intrusive or damaging to set it silently? Would the user really want to know about it?

Env variables set in the script only live in the script's execution environment and don't persist in the shell after execution so this should be fine.

  1. Can we check under what conditions it needs to be set and only set it under those (Gemini tells me it's needed when running serverless, and I bet there's a way to detect that).

For sure, I'll try to narrow down these conditions to see where it would be best to set it (in case we go with option 2)

aliberts commented 5 months ago

Something like this should do ok: https://github.com/huggingface/gym-xarm/pull/1/commits/4c65f3ed8c7aeb938cc42a3c8af7b31a319c10ef I'll add it to the other envs if needed

Cadene commented 4 months ago

Is it addressed? @aliberts

aliberts commented 4 months ago

Yes, in gym-xarm (the other envs don't use the same rendering engine) Reopening this as tests don't pass without MUJOCO_GL=egl