Split task space control from sim control#86
Conversation
alberthli
left a comment
alberthli
left a comment
There was a problem hiding this comment.
Seems like a pretty reasonable set of changes, and I tried out the test "3d" cylinder push task on my end. Some additional minor comments before it's merged:
- add yourself and this PR to the release notes for the version coming up
- small writeup in the docs would be helpful
- remove the test cylinder push task so it doesn't get merged to main - we could derive a child class of it in a test or something to verify that we can split the control/sim spaces
| self.sim_controls = self.control(self.task.data.time) | ||
| self.task.pre_sim_step() | ||
| mj_step(self.task.sim_model, self.task.data) | ||
| self.sim_backend.sim(self.task.sim_model, self.task.data, self.sim_controls) |
There was a problem hiding this comment.
nit: i'd prefer to call this function step() to match our expectations from replacing mj_step()
There was a problem hiding this comment.
point taken on this file for testing, but we shouldn't merge this in main (and similarly, we should remove it from judo.tasks.__init__)
There was a problem hiding this comment.
You can update the docs with a short snippet on how to sample from a different space than is defined in the XML
| self.task.post_sim_step() | ||
| except ValueError: | ||
| self.sim_controls = self.control(self.task.data.time) | ||
| if self.sim_controls.shape != (self.task.nu,): |
There was a problem hiding this comment.
This change is necessary because for different backends (eg cpp) the try-catch does not work
2 participants
No description provided.