Name the PyBullet client in every env call so envs can share a process - #199
Merged
Merged
Conversation
Circuit, Laser and Switch looked up their switch joints without physicsClientId, so PyBullet answered from client 0, the first world the process built. Built after any other env (through the RoboDisco wrapper, in a test session, or in scripts/robodisco_getting_started.py) they read the wrong world and failed with "getJointState failed; invalid jointIndex" or "GetJointInfo failed.". The same omission sat in calls that never crash. Laser created its beam bodies in client 0, so the planner's world drew beams into the executing env's world, and its bookkeeping then removed bodies from its own. Coffee joined its cord segments and its plugged-in plug with constraints in client 0, and the Domino fan component read its switch joints from client 0. Each of these calls now names the env's client, as do the few calls that passed it positionally, so tests/test_pybullet_client_ids.py can require physicsClientId= on every PyBullet call that reaches a physics server. Regression tests build Circuit, Laser and Switch after Blocks in one process and check that Laser's beams appear in its own world only. The envs README and the smoke script no longer describe the failure, and the smoke script numbers its seven steps out of seven.
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The bug.
Building
robodisco/Circuit-v0,Laser-v0orSwitch-v0after any other environment in the same process failed, with "getJointState failed; invalid jointIndex" for Circuit and Switch and "GetJointInfo failed." for Laser.scripts/robodisco_getting_started.pyreset only 19 of the 22 environments.Each env owns a PyBullet client, but these three looked up their switch joints without
physicsClientId, so PyBullet answered from client 0, the first world the process built.The same omission, without a crash.
pybullet_laser.pydescribed.coffee_machine_has_plug).The fix.
Each of these calls now names the env's client, as do the five calls that passed it positionally.
tests/test_pybullet_client_ids.pyparses the package and requiresphysicsClientId=on every PyBullet call that reaches a physics server; on the old code it flags every site above.The envs README and the smoke script no longer describe the failure, and the smoke script numbers its seven steps out of seven.
Test plan
scripts/robodisco_getting_started.py: 22/22 environments reset.tests/test_pybullet_client_ids.py,tests/envs/test_pybullet_laser.py(beam bodies stay in Laser's own world) andtests/envs/test_robodisco.py::test_env_builds_after_another_env.This is the first of four stacked PRs fixing the environment bugs found while auditing the envs README in #197: this one, #200 (Domino and Fan), #201 (Grow's and Coffee's NSRT samplers) and #202 (Coffee's cups).
🤖 Generated with Claude Code