Skip to content

[Offload] Add print_offload_folder util - #738

Open
kylesayrs wants to merge 3 commits into
mainfrom
kylesayrs/print-offload-util
Open

[Offload] Add print_offload_folder util#738
kylesayrs wants to merge 3 commits into
mainfrom
kylesayrs/print-offload-util

Conversation

@kylesayrs

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c0ce56d6-5318-461b-9933-9271da1181be

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A new public helper function print_offload_folder(model, offload_folder) is added to the offload module. It uses disable_onloading() and DiskCache._get_ct_file_path to map disk files to parameter metadata, printing a formatted table of known parameters and listing any unknown files. The function is exported via __all__ and documented in the offload README.

Changes

print_offload_folder utility and docs

Layer / File(s) Summary
Imports, export, and function implementation
src/compressed_tensors/offload/__init__.py
Adds os and DiskCache to imports, registers print_offload_folder in __all__, and implements the function: under disable_onloading(), it builds a mapping from expected offload file paths (via DiskCache._get_ct_file_path) to parameter path/shape/dtype, then iterates os.listdir(offload_folder) to print a formatted table for known files and collect unknown files.
README documentation
src/compressed_tensors/offload/README.md
Adds a new API section for print_offload_folder with a usage example, sample output table, and a "Use when" note for disk-offload debugging.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Provide a description explaining the purpose and use case of the new print_offload_folder utility function.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a new print_offload_folder utility function to the offload module.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kylesayrs/print-offload-util

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped vllm-project/llm-compressor.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify

mergify Bot commented Jun 18, 2026

Copy link
Copy Markdown

The quality checks have failed. Please run make style and make quality under
the root directory to adddress the lint failures. You will need to install the
dev optional install to get the required linting packages.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/compressed_tensors/offload/__init__.py (1)

323-334: ⚡ Quick win

Make output deterministic by sorting directory entries.

os.listdir(offload_folder) is not ordered, so table/unknown-file output can vary across runs. Sorting improves debuggability and reproducibility.

Suggested diff
-    for file_name in os.listdir(offload_folder):
+    for file_name in sorted(os.listdir(offload_folder)):
         file_path = os.path.join(offload_folder, file_name)
         if file_path in file_path_to_param_info:
             info = file_path_to_param_info[file_path]
             print(f"{info['path']:<60} {str(info['shape']):<25} {info['dtype']:<10}")
         else:
             unknown_paths.append(file_path)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/compressed_tensors/offload/__init__.py` around lines 323 - 334, The
os.listdir(offload_folder) call returns entries in non-deterministic order,
causing the output table and unknown files list to vary across runs. Wrap the
os.listdir() call with the sorted() function to ensure the file_name iteration
happens in alphabetical order, making the printed output deterministic and
reproducible for debugging purposes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/compressed_tensors/offload/README.md`:
- Around line 534-541: The code fence block displaying the parameter table
(containing Parameter Path, Shape, and Dtype columns) is missing a language tag
on the opening fence. Add the language identifier `text` immediately after the
opening triple backticks (change ` ``` ` to ` ```text `) to satisfy markdown
linting requirements and ensure consistent rendering.

---

Nitpick comments:
In `@src/compressed_tensors/offload/__init__.py`:
- Around line 323-334: The os.listdir(offload_folder) call returns entries in
non-deterministic order, causing the output table and unknown files list to vary
across runs. Wrap the os.listdir() call with the sorted() function to ensure the
file_name iteration happens in alphabetical order, making the printed output
deterministic and reproducible for debugging purposes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 52a32b54-bb4c-4e18-b1b4-6ce1e1ae5967

📥 Commits

Reviewing files that changed from the base of the PR and between 21c06ff and 5c392af.

📒 Files selected for processing (2)
  • src/compressed_tensors/offload/README.md
  • src/compressed_tensors/offload/__init__.py

Comment thread src/compressed_tensors/offload/README.md
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@mergify mergify Bot removed the quality-failed label Jun 22, 2026

@brian-dellabetta brian-dellabetta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cool

@kylesayrs
kylesayrs enabled auto-merge (squash) August 13, 2026 18:15
@kylesayrs kylesayrs added the ready When a PR is ready for full CI testing before merge label Aug 13, 2026
@mergify

mergify Bot commented Aug 17, 2026

Copy link
Copy Markdown

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Require one maintainer review

All PRs must have at least one approving review from a maintainer before merging.

  • #changes-requested-reviews-by = 0
  • any of:
    • approved-reviews-by=brian-dellabetta
    • approved-reviews-by=HDCharles
    • approved-reviews-by=dsikka
    • approved-reviews-by=kylesayrs

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

Labels

ready When a PR is ready for full CI testing before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants