Skip to content

Fix the options not being propagated#34

Closed
SiegeLordEx wants to merge 1 commit intoURLab-Sim:mainfrom
SiegeLordEx:fix_option_propagation
Closed

Fix the options not being propagated#34
SiegeLordEx wants to merge 1 commit intoURLab-Sim:mainfrom
SiegeLordEx:fix_option_propagation

Conversation

@SiegeLordEx
Copy link
Copy Markdown
Contributor

@SiegeLordEx SiegeLordEx commented Apr 21, 2026

Summary

Make options specified in the model XML be actually propagated to the compiled XML.

This was primarily written by Claude.

Motivation

Some models set options, such as impratio, that materially affect the simulation quality. This change lets the models work exactly as they do in the mujoco viewer without using the manager to override those options.

Build + test evidence

=== URLab build+test summary ===
Timestamp : 2026-04-21 00:04:42 UTC
Host      : instance-3
Git HEAD  : a7eb0532 (fix_option_propagation)
Engine    : C:\Program Files\Epic Games\UE_5.7
Build     : Succeeded
Tests     : 175 / 175 passed (0 failed)  [175 tests performed]
Log       : REDACTED

Manual verification steps

  1. Replace the mujoco menagerie's arx_l5 xml with the attached version (it adds two extra cubes)
  2. Add the xml to a project with some sort of ground plane
  3. Try to grab the boxes with the arm, they will slide out without this change (because the impratio is ignored)

Checklist

  • Builds locally against UE 5.7+
  • Full URLab.* automation suite passes
  • Docs updated for user-facing changes

@jonathanembleyriches
Copy link
Copy Markdown
Contributor

Happy to investigate replacing the SimOptions.ApplyToSpec(m_ChildSpec) call in AMjArticulation::Setup — it was left in place for completeness even though we knew it gets dropped by mjs_attach, with all option-overriding routed through the Manager's FMuJoCoOptions instead. That was a design decision we made but didn't document, and not necessarily the right one.

The main challenge is the multi-articulation conflict. MuJoCo has one global option block per compiled model, so in scenes with more than one articulation we need a defensible precedence rule when their <option> settings disagree.

A couple of things worth noting:

  • In practice, conflicts may be less common than they look. FMuJoCoOptions uses bOverride_* flags, so if we switch to a gated apply (only write fields where the flag is true), articulations will only touch options their MJCF actually specified. Most articulations probably don't set the same fields as others, so real conflicts will be rarer than a naive "every articulation overwrites everything" would suggest. Still possible though, and we need a rule for when it happens.

  • Last-wins and first-wins are both just scene-iteration order. UE actor iteration order isn't guaranteed stable when you re-save the level, so both are fragile.

  • A per-articulation priority field (an explicit int UPROPERTY) is probably the cleanest option. It makes precedence user-controlled and deterministic across saves, at the cost of one more field per articulation. (although we also have to decide when two articulations share the same priority)

The current PR picks first-wins implicitly by only applying m_articulations[0], which would break silently when a user adds a second articulation. That's the part that needs thinking through before we merge anything.

The Manager's SimOptions should still run post-compile as the final decision. That's intentional and we want to keep it. Per-articulation MJCF options would be the baseline, and the scene-wide Manager override always wins on top.

Rough shape I'm thinking:

  1. Gated apply so each articulation only writes fields where bOverride_* == true.
  2. Deterministic precedence via a priority field or similar. (happy to hear any better ideas on the best strategy for this)
  3. Loud warning on conflict so users know when their precedence setting actually mattered.
  4. Manager's SimOptions overrides continue to trump everything post-compile.

Does that direction sound right to you? Are you happy to sketch the priority-field approach (or any other deterministic strategy) in a follow-up commit on your branch, or do you want me to take a look at it?

@SiegeLordEx
Copy link
Copy Markdown
Contributor Author

The priority system sounds complicated. Honestly, I'd be fine with detecting that the option is set in the XML and complaining loudly that it is ignored. It seems to be if you expect multiple articulations as a typical use case, then you can't really expect a MuJoCo XML file to work without any modification.

@jonathanembleyriches
Copy link
Copy Markdown
Contributor

In most cases I don't think xml files (and therefore our internal articulations) will typically have conflicting options. But, for the sake of robustness I I'll draft up a PR for the priority system.

I'm planning on treating it similar to MuJoCo's own internal priority system for contacts + physics where whatever (geoms) in our case (articulations) have conflicting parameters the highest takes priority. We will have a loud warning when this takes place so it is clear to the user. Users can feel free to ignore this option as we will default priority to 0 and as a fallback the first articulation will win. It is then up to the user to tweak any overides themselves to get their required behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants