mujoco icon indicating copy to clipboard operation
mujoco copied to clipboard

`MjSpec.compile()` should populate resulting `MjModel`'s XML to enable `mujoco.mj_saveLastXML`

Open hartikainen opened this issue 1 year ago • 1 comments

The feature, motivation and pitch

Currently, the mj_model resulting from mj_model = mj_spec.compile() cannot be saved with mujoco.mj_saveLastXML(mj_model) because the XML information is not stored in the model. It would be useful to be able to save the XML of an MjModel without carrying the MjSpec around.

It feels like this would be as simple as storing the MjSpec.to_xml() into the MjModel, but perhaps I'm overlooking something and this is actually more complicated than I think.

Alternatives

I can save the compiled model with mujoco.mj_saveModel. For debugging purposes, it would be nice to be able to look into the XML. This I can also do by keeping the spec around but it would be more convienient to not have to worry about the spec.

Additional context

Here's a simple test case that fails:

  def test_spec_model_saveLastXML(self):
    spec_1 = mujoco.MjSpec()
    spec_2 = mujoco.MjSpec()
    spec_1.from_string("<mujoco></mujoco>")
    model = spec_1.compile()
    with tempfile.NamedTemporaryFile(suffix=".xml", mode="w+t") as fp:
      mujoco.mj_saveLastXML(fp.name, model)
      spec_2.from_file(fp.name)

    self.assertEqual(spec_1.to_xml(), spec_2.to_xml())

Which gives:

======================================================================
ERROR: test_spec_model_saveLastXML (specs_test.SpecsTest.test_spec_model_saveLastXML)
specs_test.SpecsTest.test_spec_model_saveLastXML
----------------------------------------------------------------------
Traceback (most recent call last):
  File "mujoco/python/mujoco/specs_test.py", line 802, in test_spec_model_saveLastXML
    mujoco.mj_saveLastXML(fp.name, model)
mujoco.FatalError: No XML model loaded

----------------------------------------------------------------------

hartikainen avatar Sep 22 '24 09:09 hartikainen

In what way is mj_saveXML not what you want?

yuvaltassa avatar Sep 27 '24 13:09 yuvaltassa

In what way is mj_saveXML not what you want?

I have the same issue, the problem is that, mj_saveXML is not binded in python. There are only:

def mj_saveLastXML(filename, m): # real signature unknown; restored from __doc__
    """
    mj_saveLastXML(filename: str, m: mujoco._structs.MjModel) -> None
    
    Update XML data structures with info from low-level model, save as MJCF. If error is not NULL, it must have size error_sz.
    """
    pass

def mj_saveModel(m, filename, p_str=None, *args, **kwargs): # real signature unknown; NOTE: unreliably restored from __doc__ 
    """
    mj_saveModel(m: mujoco._structs.MjModel, filename: Optional[str] = None, buffer: Optional[numpy.ndarray[numpy.uint8[m, 1], flags.writeable]] = None) -> None
    
    Save model to binary MJB file or memory buffer; buffer has precedence when given.
    """
    pass

HoangGiang93 avatar Dec 08 '24 12:12 HoangGiang93

This is addressed in the notebook. This notebook is not yet finished so we haven't advertised it, but I believe everything should work.

@quagla we should also add a subsection about XML saving to the docs...

yuvaltassa avatar Dec 08 '24 22:12 yuvaltassa

@hartikainen the feature request is not entirely clear, TBH. mj_saveLastXML is only relevant for things that were loaded using the old API mj_loadXML, where there is only one global model that was "last loaded". It doesn't really make sense in the mjSpec world. Why is "carrying the MjSpec around" a problem for you?

I'm closing this for now since I don't really understand the feature request :slightly_smiling_face:

Feel free to keep explaining here or open a completely new feature request, if you like.

yuvaltassa avatar Dec 10 '24 15:12 yuvaltassa