deepseek-ai/DeepEP
1
0
Fork 0
mirror of https://github.com/deepseek-ai/DeepEP.git synced 2026-11-05 00:47:32 +00:00
Code Issues 308 Projects Packages Wiki Activity

[GH-ISSUE #718] EP_BUFFER_DEBUG=0 enables Python-side debug output because the env var is tested as a raw string #411

New issue
Open
opened 2026-10-26 10:00:14 +00:00 by bsakaguchi · 1 comment
bsakaguchi commented 2026-10-26 10:00:14 +00:00
Owner
Copy link

Originally created by @yashkgp on GitHub (Aug 9, 2026).
Original GitHub issue: https://github.com/deepseek-ai/DeepEP/issues/718

Summary

EP_BUFFER_DEBUG=0 — the value the README documents as "off" — turns the Python-side debug output on, while the C++ side correctly stays off. The Python code tests the raw environment string for truthiness, and the string '0' is truthy in Python.

Details

The README documents the flag as a 0/1 toggle, defaulting to 0:

  • EP_BUFFER_DEBUG: 0 or 1, print buffer initialization, SM approximation, and backend debugging information, 0 by default

The C++ side honors that contract — it parses the value as an int before testing it:

// csrc/kernels/backend/nccl.cu:38, :45, :79
// csrc/elastic/buffer.hpp:865, :1055
if (get_env<int>("EP_BUFFER_DEBUG"))

get_env<int> runs the value through sscanf(c_str, "%d", &value) (csrc/utils/system.hpp:28-31), so "0" becomes 0 — falsy. Correct.

The Python side never converts, and tests the string directly:

# deep_ep/buffers/elastic.py:311
if os.environ.get('EP_BUFFER_DEBUG', 0):
    print(f'Initializing EP elastic buffer with {num_bytes} bytes ...')

# deep_ep/buffers/elastic.py:828
if os.environ.get('EP_BUFFER_DEBUG', 0):
    print(f'EP SM approximation: ...')

With EP_BUFFER_DEBUG=0, os.environ.get(...) returns the string '0', which is truthy — so both print calls fire.

Note the default value 0 (an int) is never actually what gets tested when the variable is set; it only applies when the variable is absent. That is why the unset case behaves correctly and only the explicitly-disabled case misbehaves.

Reproduction

import os
os.environ['EP_BUFFER_DEBUG'] = '0'

print('python side  ->', bool(os.environ.get('EP_BUFFER_DEBUG', 0)))   # matches elastic.py
print('cpp side     ->', bool(int(os.environ.get('EP_BUFFER_DEBUG', 0))))  # matches get_env<int>

python side  -> True
cpp side     -> False

Any non-empty value behaves the same way, so EP_BUFFER_DEBUG=false and EP_BUFFER_DEBUG=off also enable the Python output.

Expected vs. actual

EP_BUFFER_DEBUG Expected Python actual C++ actual
unset off off off
0 off on off
1 on on on

The two halves of a single documented flag disagree, and explicitly disabling it is the one case that breaks.

Impact

Minor but user-visible: a user who explicitly disables the flag gets per-rank print output on every ElasticBuffer construction and every get_theoretical_num_sms call, with no way to turn it off short of unsetting the variable. Explicitly setting a flag to its documented off value is a common thing to do in launcher scripts and config templates, where variables are usually set unconditionally rather than conditionally omitted.

Suggested fix

Convert before testing, which is the idiom this repo already uses at every other EP_* boolean flag:

if int(os.environ.get('EP_BUFFER_DEBUG', 0)):

For reference, the existing call sites that already do this:

  • deep_ep/__init__.py:51 — int(os.environ.get('EP_SUPPRESS_NCCL_CHECK', 0))
  • deep_ep/utils/testing.py:143 — int(os.environ.get('EP_USE_NVIDIA_TOOLS', 0))
  • deep_ep/utils/testing.py:152 — int(os.environ.get('EP_DISABLE_BARRIER_PROFILING', 0))
  • deep_ep/utils/comm.py:62 — int(os.getenv('EP_REUSE_NCCL_COMM', '1'))
  • setup.py:130, setup.py:153 — same pattern

EP_BUFFER_DEBUG at elastic.py:311 and elastic.py:828 are the only two boolean env reads in the package that omit the conversion.

I'd be happy to send a PR for this.

Originally created by @yashkgp on GitHub (Aug 9, 2026). Original GitHub issue: https://github.com/deepseek-ai/DeepEP/issues/718 ## Summary `EP_BUFFER_DEBUG=0` — the value the README documents as "off" — turns the **Python-side** debug output **on**, while the C++ side correctly stays off. The Python code tests the raw environment string for truthiness, and the string `'0'` is truthy in Python. ## Details The README documents the flag as a `0`/`1` toggle, defaulting to `0`: > - `EP_BUFFER_DEBUG`: `0` or `1`, print buffer initialization, SM approximation, and backend debugging information, `0` by default The C++ side honors that contract — it parses the value as an `int` before testing it: ```cpp // csrc/kernels/backend/nccl.cu:38, :45, :79 // csrc/elastic/buffer.hpp:865, :1055 if (get_env<int>("EP_BUFFER_DEBUG")) ``` `get_env<int>` runs the value through `sscanf(c_str, "%d", &value)` (`csrc/utils/system.hpp:28-31`), so `"0"` becomes `0` — falsy. Correct. The Python side never converts, and tests the string directly: ```python # deep_ep/buffers/elastic.py:311 if os.environ.get('EP_BUFFER_DEBUG', 0): print(f'Initializing EP elastic buffer with {num_bytes} bytes ...') # deep_ep/buffers/elastic.py:828 if os.environ.get('EP_BUFFER_DEBUG', 0): print(f'EP SM approximation: ...') ``` With `EP_BUFFER_DEBUG=0`, `os.environ.get(...)` returns the string `'0'`, which is truthy — so both `print` calls fire. Note the default value `0` (an `int`) is never actually what gets tested when the variable is set; it only applies when the variable is absent. That is why the unset case behaves correctly and only the explicitly-disabled case misbehaves. ## Reproduction ```python import os os.environ['EP_BUFFER_DEBUG'] = '0' print('python side ->', bool(os.environ.get('EP_BUFFER_DEBUG', 0))) # matches elastic.py print('cpp side ->', bool(int(os.environ.get('EP_BUFFER_DEBUG', 0)))) # matches get_env<int> ``` ``` python side -> True cpp side -> False ``` Any non-empty value behaves the same way, so `EP_BUFFER_DEBUG=false` and `EP_BUFFER_DEBUG=off` also enable the Python output. ## Expected vs. actual | `EP_BUFFER_DEBUG` | Expected | Python actual | C++ actual | |---|---|---|---| | unset | off | off | off | | `0` | off | **on** | off | | `1` | on | on | on | The two halves of a single documented flag disagree, and explicitly disabling it is the one case that breaks. ## Impact Minor but user-visible: a user who explicitly disables the flag gets per-rank `print` output on every `ElasticBuffer` construction and every `get_theoretical_num_sms` call, with no way to turn it off short of unsetting the variable. Explicitly setting a flag to its documented off value is a common thing to do in launcher scripts and config templates, where variables are usually set unconditionally rather than conditionally omitted. ## Suggested fix Convert before testing, which is the idiom this repo already uses at every other `EP_*` boolean flag: ```python if int(os.environ.get('EP_BUFFER_DEBUG', 0)): ``` For reference, the existing call sites that already do this: - `deep_ep/__init__.py:51` — `int(os.environ.get('EP_SUPPRESS_NCCL_CHECK', 0))` - `deep_ep/utils/testing.py:143` — `int(os.environ.get('EP_USE_NVIDIA_TOOLS', 0))` - `deep_ep/utils/testing.py:152` — `int(os.environ.get('EP_DISABLE_BARRIER_PROFILING', 0))` - `deep_ep/utils/comm.py:62` — `int(os.getenv('EP_REUSE_NCCL_COMM', '1'))` - `setup.py:130`, `setup.py:153` — same pattern `EP_BUFFER_DEBUG` at `elastic.py:311` and `elastic.py:828` are the only two boolean env reads in the package that omit the conversion. I'd be happy to send a PR for this.
bsakaguchi commented 2026-10-26 10:00:16 +00:00
Author
Owner
Copy link

@YidaWeng commented on GitHub (Oct 1, 2026):

I would like to refresh the fix for this on current main.

#719 and #742 both target deep_ep/buffers/elastic.py, which was removed in the V2.5 split (ElasticBuffer → EPBuffer). The bug is still present in deep_ep/buffers/ep.py (the two os.environ.get('EP_BUFFER_DEBUG', 0) truth tests).

Plan:

  • wrap both call sites with int(...), matching the existing EP_* boolean idiom and C++ get_env<int>
  • add a host-side regression test under tests/utils/ (no CUDA / no compiled extension)
  • open a small PR against current main

Is anyone actively rebasing #719 / #742, or is a fresh PR against ep.py preferred?

<!-- gh-comment-id:5929388029 --> @YidaWeng commented on GitHub (Oct 1, 2026): I would like to refresh the fix for this on current `main`. `#719` and `#742` both target `deep_ep/buffers/elastic.py`, which was removed in the V2.5 split (`ElasticBuffer` → `EPBuffer`). The bug is still present in `deep_ep/buffers/ep.py` (the two `os.environ.get('EP_BUFFER_DEBUG', 0)` truth tests). Plan: - wrap both call sites with `int(...)`, matching the existing `EP_*` boolean idiom and C++ `get_env<int>` - add a host-side regression test under `tests/utils/` (no CUDA / no compiled extension) - open a small PR against current `main` Is anyone actively rebasing `#719` / `#742`, or is a fresh PR against `ep.py` preferred?
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
hybrid-ep
chore/remove-clang-format-yapf
v2.5
nvDev
try-fix-cu128
mori-ep
antgroup-opt
compat-old-torch
add-linter
eager-rdma
v1.2.1
Labels
Clear labels   
pull-request
Mirrored from GitHub Pull Request

No labels
pull-request
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
deepseek-ai/DeepEP#411
No description provided.