Skip to content

Fix NuPlan scene-to-asset-folder mapping for MTGS renderer#111

Open
WCJ-BERT wants to merge 3 commits into
NVlabs:e2e_challengefrom
WCJ-BERT:e2e_challenge
Open

Fix NuPlan scene-to-asset-folder mapping for MTGS renderer#111
WCJ-BERT wants to merge 3 commits into
NVlabs:e2e_challengefrom
WCJ-BERT:e2e_challenge

Conversation

@WCJ-BERT

Copy link
Copy Markdown

Multiple scene tokens sharing the same MTGS road block config were each trying to resolve their own asset folder (named after their unique token), which only exists for the first token (road_block_name).

Fix via two complementary layers:

  1. Runtime resolver in main.py (no cache regen required): at server startup, read all MTGS config YAMLs from the configs/ directory and build a token → road_block_name mapping. This is injected as asset_folder_resolver into TrajdataDataSource so any token resolves to its shared asset folder regardless of cache state.

  2. Cache pipeline fix in scene_loader.py + nuplan_utils.py: propagate road_block_name as asset_folder through central_tokens_config → scene record → data_access_info so newly built caches also carry the correct mapping.

Also add nuplan_private → private to DATASET_NAME_MAPPING in the MTGS server.

@copy-pr-bot

copy-pr-bot Bot commented Jun 25, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

"%s missing central_log or central_tokens, skipping", yaml_file.name
)
continue
road_block_name = (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since cfg comes from yaml.load(), can we simplify this to: cfg.get("central_log", "") or f"{central_log}-{central_tokens[0]}"

@jarcherNV

Copy link
Copy Markdown
Collaborator

This looks directionally right, but I think _build_token_to_asset_folder() should use the same fallback as _resolve_nuplan_extra_params().

Right now the cache path sets asset_folder to road_block_name or f"{central_log}-{central_tokens[0]}", but the MTGS runtime resolver only maps tokens when road_block_name is already present in the YAML. For existing configs without road_block_name, the mapping will be empty and the runtime will fall back to scene.name, which seems to preserve the original failure mode.

Can we read central_log here too and use:

road_block_name = cfg.get("road_block_name", "") or f"{central_log}-{central_tokens[0]}"

when both central_log and central_tokens are available?

Could we also add a small test for _build_token_to_asset_folder() covering a YAML with central_log and multiple central_tokens but no explicit road_block_name? That would lock in the no-cache-regeneration behavior this PR is aiming for.

WCJ-BERT and others added 3 commits July 13, 2026 12:33
Multiple scene tokens sharing the same MTGS road block config were each
trying to resolve their own asset folder (named after their unique token),
which only exists for the first token (road_block_name).

Fix via two complementary layers:

1. Runtime resolver in main.py (no cache regen required): at server startup,
   read all MTGS config YAMLs from the configs/ directory and build a
   token → road_block_name mapping.  This is injected as asset_folder_resolver
   into TrajdataDataSource so any token resolves to its shared asset folder
   regardless of cache state.

2. Cache pipeline fix in scene_loader.py + nuplan_utils.py: propagate
   road_block_name as asset_folder through central_tokens_config → scene
   record → data_access_info so newly built caches also carry the correct
   mapping.

Also add nuplan_private → private to DATASET_NAME_MAPPING in the MTGS server.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yaml.load() always returns a dict, so the isinstance/getattr branch is
redundant. Use cfg.get() directly, per code review suggestion.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@WCJ-BERT

Copy link
Copy Markdown
Author

Thanks for the suggestion! Updated the fallback and added a regression test.

@jarcherNV

Copy link
Copy Markdown
Collaborator

Thanks, I will trigger CI.

@jarcherNV

Copy link
Copy Markdown
Collaborator

/ok to test ce985d2

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants