Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mj_copyData not copying contact details? #1710

Closed
marioprats opened this issue Jun 3, 2024 · 4 comments
Closed

mj_copyData not copying contact details? #1710

marioprats opened this issue Jun 3, 2024 · 4 comments
Assignees
@nimrod-gileadi
Labels
bug Something isn't working

Comments

@marioprats
Copy link

Hi,
I have an issue with mj_copyData not copying contact details. Not sure if this is a bug or something intended.

Here is a minimal model:

minimal XML 
<mujoco>
  <worldbody>
    <light pos=".4 -.4 .3" dir="-2 2 -1.5" diffuse=".6 .6 .6"/>
    <light pos="-.2 -.4 .3" dir="1 2 -1.5" diffuse=".6 .6 .6"/>
    <geom type="plane" size=".5 .5 .01"/>
    <body name="ball" pos="0 0 0.0">
      <freejoint/>
      <geom size=".03"/>
    </body>
  </worldbody>
</mujoco>

It has a sphere in contact with a ground plane.
I found the issue with mj_copyData in C++, but it can be reproduced in python too:

minimal Python reproduction case 
import mujoco
import copy

model = mujoco.MjModel.from_xml_path("scene.xml")
data = mujoco.MjData(model)

mujoco.mj_forward(model,data)

print("Original data:")
print("  n_contacts: ", data.ncon)
print("  colliding geoms: ", data.contact.geom)

data2 = copy.copy(data)

print("Data copy:")
print("  n_contacts: ", data2.ncon)
print("  colliding geoms: ", data2.contact.geom)

The output is:

Original data:
  n_contacts:  1
  colliding geoms:  [[0 1]]
Data copy:
  n_contacts:  1
  colliding geoms:  [[0 0]]

So the copy preserves ncon, but doesn't seem to copy contact details over (e.g. colliding geoms).
@marioprats marioprats added the question Request for help or information label Jun 3, 2024
@yuvaltassa
Copy link
Collaborator

Hmmm, this should work. (That said, there is no test for this function, so ya this could be a bug)

@saran-t is there a bug in this line?

@saran-t
Copy link
Member

saran-t commented Jun 3, 2024

It's certainly not deep-copying the arena. I can't remember whether that was a deliberate choice though, for MJPC?

@yuvaltassa
Copy link
Collaborator

Oh damn you're right. So the comment // copy arena memory is basically a lie eh?
It should either be // set arena pointers, don't copy memory or we should actually copy the memory...

I have no recollection of this being a deliberate choice, seems like a strange thing to do. @nimrod-gileadi, @erez-tom, @thowell, any such recollection?

Marking this as a bug, even if we end up deciding that it's deliberate, the inline and API docs should be updated.

@yuvaltassa yuvaltassa added bug Something isn't working and removed question Request for help or information labels Jun 3, 2024
@nimrod-gileadi
Copy link
Collaborator

We do have tests for this function in the Python bindings, but indeed we don't check for contacts:

mujoco/python/mujoco/bindings_test.py

Line 430 in 14a3ce5

data_copy = copy.copy(self.data)

I think that copying the arena content is the right thing to do. Can't see why not.

@nimrod-gileadi nimrod-gileadi self-assigned this Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nimrod-gileadi nimrod-gileadi

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@marioprats @saran-t @yuvaltassa @nimrod-gileadi