[AMDGPU] Fix: increase Quadrants device memory pool on ROCm (hipMemset)#3065
[AMDGPU] Fix: increase Quadrants device memory pool on ROCm (hipMemset)#3065gayathiri-venkataraman wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcfb4567f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| qd_debug = debug and (os.environ.get("QD_DEBUG") != "0") | ||
| # Set Quadrants device memory pool size. The default (1 GB) is far too small for large-scale simulations. | ||
| # Use the GS_DEVICE_MEMORY_GB env var if set, otherwise allocate up to 80% of total device memory. | ||
| if backend != _gs_backend.cpu: |
There was a problem hiding this comment.
Limit the pool expansion to AMDGPU
This branch now runs for every non-CPU backend, including CUDA and Metal selected by init, even though the change is meant to address ROCm hipMemset failures. On CUDA/Metal jobs that already rely on PyTorch, rendering, or other processes sharing VRAM, raising Quadrants from its previous 1 GB default to 80% of device memory at gs.init() can starve those allocations before any ROCm workload is involved; guard this to _gs_backend.amdgpu or make it opt-in for the other GPU backends.
Useful? React with 👍 / 👎.
| if _device_mem_gb_env is not None: | ||
| qd_init_kwargs["device_memory_GB"] = float(_device_mem_gb_env) | ||
| elif "device_memory_GB" not in qd_init_kwargs: | ||
| qd_init_kwargs["device_memory_GB"] = round(total_mem * 0.8, 1) |
There was a problem hiding this comment.
Use GPU memory before forcing Torch to CPU
When GS_TORCH_FORCE_CPU_DEVICE=1 is set with a GPU backend, the fallback above overwrites total_mem with host RAM while leaving backend as the GPU backend. This new default then passes round(total_mem * 0.8, 1) as Quadrants GPU device_memory_GB, so machines with much more system RAM than VRAM can request an impossible GPU pool and fail initialization; preserve the original GPU total or skip this override in that mode.
Useful? React with 👍 / 👎.
|
I dont have a major concern with enabling an environment variable to control this, so if you make this PR just about that, I'm fairly comfortable merging that. Changing the default allocation is much more concerning to me, and would need a lot more investigation and discussion, since you are the only person who has raised this issue, and memory allocation is a big deal. Are you able to consider scoping this PR to just honoring the env var, or do you want to initiate a longer discussion about the goals and use-cases for the change in the default? My concerns about changing the default by the way are:
|
|
Ran your mem script on MI300X: default, QD_DEVICE_MEMORY_GB=8, and GS_DEVICE_MEMORY_GB=8 all grab ~1.24 GB at init (pool path) — env vars don't change allocation here; dropped the 80% default, PR now AMDGPU-only GS_DEVICE_MEMORY_GB when set. |
|
Ok, so we are concurring that So the problem you are facing lies elsewhere. Could you post a minimum reproducible example script to reprdocue your issue please? (I have access to an MI300X I can run your reproduction script on). |







Summary
On ROCm/AMDGPU, Genesis init now sets Quadrants
device_memory_GBfromGS_DEVICE_MEMORY_GBwhen set, otherwise ~80% of total device memory. The default 1 GB pool is too small for large-scale sims and can cause hipMemset allocation failures.Also adjusts the default
PYOPENGL_PLATFORMhandling in the rasterizer so an empty env var does not force an invalid platform string on headless nodes.Correctness
No numerical/physics change — memory pool sizing and renderer platform selection only.
Verification
Tested on AMD MI300X/MI325X (gfx942) vs baseline.
Made with Cursor