Repository navigation
Conversation
3dae18d to
3c0542c
Compare
|
yuecideng
left a comment
There was a problem hiding this comment.
Reviewed 5c62b0b...8ffc94b. Two P2 findings are attached inline. The 12 benchmark unit tests passed, all six CUDA cases collected, and both reported behaviors were reproduced with CPU-only probes using the pinned Python helpers. Live CUDA/simulation and real NVIDIA process-memory sampling were not rerun.
| if status.is_file(): | ||
| for line in status.read_text().splitlines(): | ||
| if line.startswith("NSpid:"): | ||
| self.pids.update(int(value) for value in line.split()[1:]) |
There was a problem hiding this comment.
[P2] Resolve the PID namespace before matching process VRAM
Adding every NSpid value to the match set mixes identifiers from different PID namespaces. When nvidia-smi reports host PIDs, a worker with NSpid 4300 73 can also match an unrelated same-GPU host process numbered 73. _process_gpu_mib() then takes the maximum of both records. A CPU probe using worker row 4300, 1000, GPU-a and unrelated row 73, 7000, GPU-a reports 7000 MiB for the worker instead of 1000 MiB, silently corrupting the backend memory comparison. Match only the worker PID confirmed in the sampling provider's namespace, and preserve an unknown value if that identity cannot be resolved.
There was a problem hiding this comment.
Fixed in a9a1452a. Sampling now matches one confirmed host PID and GPU UUID, preserving unknown values when host identity cannot be resolved. The regression returns the worker's 1000 MiB and excludes the colliding process. Real Linux namespace probes pass, and all seven NVIDIA memory samples are valid in the Newton run against current main.
| trainer.global_step = saved["global_step"] | ||
| trainer.num_updates = saved["num_updates"] | ||
| trainer.best_eval_value = saved["best_eval_value"] | ||
| summary = trainer.train(trainer.global_step + 8 * 24) |
There was a problem hiding this comment.
[P2] Refresh the collector observation after the inference rollout
_build_trainer() above constructs a SyncCollector that immediately resets the environment and caches its observations. The subsequent inference check resets and steps the same environment 32 times without updating that cache, so this train() call starts its first resumed transition from a stale observation while the simulator is in its current state. G1 builds fresh actor/critic observation tensors, and the official PPO configuration does not reset every rollout, so aliasing or an automatic reset does not repair the mismatch. A CPU probe with the production collector reproduced initial rollout observation 0 paired with an actual environment state of 32. Finite losses, changed parameters, and increasing Adam step counters can still pass. Recreate or synchronize the collector after inference and before resumed training.
There was a problem hiding this comment.
Fixed in a9a1452a. Before resumed training, the collector cache is synchronized with observations returned by the final inference step. All three Newton integration cases pass against current main. The checkpoint test directly verifies exact actor/critic observations in the first resumed transition; parameter, optimizer and training-counter checks also pass.
Description
Add a G1 PPO benchmark using the existing EmbodiChain policy runtime and trainer, with Default CUDA and Newton/MJWarp CUDA backend selection. The standard workload is 4,096 environments, 24 rollout steps, 3 warmup updates and 10 measured updates. Save JSON/Markdown results and a training checkpoint; report creation, rollout and optimizer time, throughput variation, process RAM and sampled process VRAM.
Add CUDA tests for G1/Go2 inference and partial reset, including preservation of unselected joint/root states, plus G1 checkpoint restoration and resumed training in a fresh process. Match process VRAM to one confirmed host PID, preserving unknown samples when namespace identity cannot be resolved, and synchronize collector observations after checkpoint inference. Preserve resource samples when runtime construction fails. Workers resolve the checkout path and use the public task-discovery/init-hook entry points, so they also run with an editable installation. This PR adds benchmark and verification infrastructure while retaining the existing application-side simulation and PPO implementation.
Dependencies: published
dexsim-engine==0.5.1rc1.Validation
All six CUDA checks for G1/Go2 inference, partial reset and fresh-process checkpoint resume passed, including editable-install execution. The current head also passed 17 benchmark unit tests and all three Newton integration cases against current
main, verifying exact actor/critic observations in the first resumed transition and valid NVIDIA memory samples at all seven phase boundaries. Real Linux PID namespace probes preserve unknown values when host identity is unavailable. With the published wheel, the Newton/MJWarp CUDA Hybrid benchmark completed 13 PPO updates at 4,096 G1 environments × 24 steps, with finite rollout/losses, parameter changes, independent policy/optimizer checkpoint restoration and exit 0.Type of change
Checklist