[PR #719] Fix EP_BUFFER_DEBUG=0 turning on Python-side debug output #730

Open
opened 2026-10-27 21:05:41 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/DeepEP/pull/719
Author: @yashkgp
Created: 8/9/2026
Status: 🔄 Open

Base: main ← Head: fix/ep-buffer-debug-env-truthiness


📝 Commits (1)

  • b3475fa Fix EP_BUFFER_DEBUG=0 turning on Python-side debug output

📊 Changes

1 file changed (+2 additions, -2 deletions)

View changed files

📝 deep_ep/buffers/elastic.py (+2 -2)

📄 Description

Fixes #718.

Problem

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 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 by converting before testing (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.

The two Python call sites tested the raw string instead, and '0' is truthy in Python:

# deep_ep/buffers/elastic.py:311, :828
if os.environ.get('EP_BUFFER_DEBUG', 0):

The int default 0 only applies when the variable is absent, which is why the unset case works and only the explicitly-disabled case misbehaves.

Fix

Convert before testing, matching get_env<int> and the idiom already used by every other boolean env flag in the package:

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

Existing call sites that already do this — deep_ep/__init__.py:51 (EP_SUPPRESS_NCCL_CHECK), deep_ep/utils/testing.py:143 (EP_USE_NVIDIA_TOOLS), :152 (EP_DISABLE_BARRIER_PROFILING), deep_ep/utils/comm.py:62 (EP_REUSE_NCCL_COMM), and setup.py:130/:153. The two lines changed here were the only boolean env reads in the package that omitted the conversion.

Verification

Host-side; the defect and the fix are both pure Python and need no device. Evaluating the exact expression at elastic.py:311/:828 before and after:

EP_BUFFER_DEBUG C++ (get_env<int>) Python before Python after
unset off off off
0 off on off
1 on on on
2 on on on

Python now agrees with the C++ side on every value. Only the 0 row changes; the unset and enabled paths are untouched.

Scope

Two lines, both inside if conditions guarding print calls. No behavior change to any code path other than the one that was contradicting its own documentation.

Note on the format-check job

CI is red here on pre-existing lint in deep_ep/buffers/elastic.py, not on anything this PR introduces. format.sh only lints files that differ from origin/main, so main never lints this file and these never surface there.

Running the pinned tooling (yapf==0.40.2, ruff==0.6.5, repo pyproject.toml) against unmodified origin/main (01dc3aa) versus this branch:

                      origin/main    this PR
yapf --diff lines         259          259
ruff errors                 3            3

Identical — this PR adds zero new violations. The 3 ruff errors on main are F401 (typing.List unused, L6) and F541 ×2 (f-strings without placeholders, L930/L1088).

I deliberately did not fold that cleanup in: the yapf delta alone is a ~259-line reformat, which would bury a 2-line bugfix. Happy to send it as a separate PR if you'd like the file made lint-clean — I notice #685 ran into the same thing and made the same offer.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/DeepEP/pull/719 **Author:** [@yashkgp](https://github.com/yashkgp) **Created:** 8/9/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/ep-buffer-debug-env-truthiness` --- ### 📝 Commits (1) - [`b3475fa`](https://github.com/deepseek-ai/DeepEP/commit/b3475fa8576c241058dae4bc0344cf583f64cf54) Fix EP_BUFFER_DEBUG=0 turning on Python-side debug output ### 📊 Changes **1 file changed** (+2 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `deep_ep/buffers/elastic.py` (+2 -2) </details> ### 📄 Description Fixes #718. ## Problem `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 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 by converting before testing (`csrc/kernels/backend/nccl.cu:38`, `:45`, `:79`; `csrc/elastic/buffer.hpp:865`, `:1055`): ```cpp 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. The two Python call sites tested the raw string instead, and `'0'` is truthy in Python: ```python # deep_ep/buffers/elastic.py:311, :828 if os.environ.get('EP_BUFFER_DEBUG', 0): ``` The `int` default `0` only applies when the variable is **absent**, which is why the unset case works and only the explicitly-disabled case misbehaves. ## Fix Convert before testing, matching `get_env<int>` and the idiom already used by every other boolean env flag in the package: ```python if int(os.environ.get('EP_BUFFER_DEBUG', 0)): ``` Existing call sites that already do this — `deep_ep/__init__.py:51` (`EP_SUPPRESS_NCCL_CHECK`), `deep_ep/utils/testing.py:143` (`EP_USE_NVIDIA_TOOLS`), `:152` (`EP_DISABLE_BARRIER_PROFILING`), `deep_ep/utils/comm.py:62` (`EP_REUSE_NCCL_COMM`), and `setup.py:130`/`:153`. The two lines changed here were the only boolean env reads in the package that omitted the conversion. ## Verification Host-side; the defect and the fix are both pure Python and need no device. Evaluating the exact expression at `elastic.py:311`/`:828` before and after: | `EP_BUFFER_DEBUG` | C++ (`get_env<int>`) | Python before | Python after | |---|---|---|---| | unset | off | off | off | | `0` | off | **on** | off | | `1` | on | on | on | | `2` | on | on | on | Python now agrees with the C++ side on every value. Only the `0` row changes; the unset and enabled paths are untouched. ## Scope Two lines, both inside `if` conditions guarding `print` calls. No behavior change to any code path other than the one that was contradicting its own documentation. ## Note on the `format-check` job CI is red here on **pre-existing** lint in `deep_ep/buffers/elastic.py`, not on anything this PR introduces. `format.sh` only lints files that differ from `origin/main`, so `main` never lints this file and these never surface there. Running the pinned tooling (`yapf==0.40.2`, `ruff==0.6.5`, repo `pyproject.toml`) against **unmodified `origin/main`** (`01dc3aa`) versus this branch: ``` origin/main this PR yapf --diff lines 259 259 ruff errors 3 3 ``` Identical — this PR adds zero new violations. The 3 ruff errors on `main` are `F401` (`typing.List` unused, L6) and `F541` ×2 (f-strings without placeholders, L930/L1088). I deliberately did not fold that cleanup in: the yapf delta alone is a ~259-line reformat, which would bury a 2-line bugfix. Happy to send it as a separate PR if you'd like the file made lint-clean — I notice #685 ran into the same thing and made the same offer. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
No labels
pull-request
No milestone
No project
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#730
No description provided.