Skip to content

Delete deprecated SemiSparseLayout and related code#4148

Merged
jerryzh168 merged 24 commits intomainfrom
gh/jerryzh168/64/head
Mar 24, 2026
Merged

Delete deprecated SemiSparseLayout and related code#4148
jerryzh168 merged 24 commits intomainfrom
gh/jerryzh168/64/head

Conversation

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Mar 23, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/4148

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@jerryzh168 jerryzh168 added the module: not user facing Use this tag if you don't want this PR to show up in release notes label Mar 23, 2026
@jerryzh168
Copy link
Copy Markdown
Contributor Author

@claude review

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

Claude finished @jerryzh168's task in 1m 7s —— View job


Review of PR #4148: Delete deprecated SemiSparseLayout and related code

  • Gather context and read PR diff
  • Review changed files
  • Post review feedback

Overall: Clean deletion PR. The removal of SemiSparseLayout, SemiSparseAQTTensorImpl, and int8_dynamic_activation_int8_semi_sparse_weight is straightforward, and the associated tests/benchmarks are properly cleaned up. LGTM with one nit below.

Nit: Leftover references to SemiSparseLayout

The following files still reference SemiSparseLayout after this PR and may warrant cleanup (possibly in a follow-up):

  1. torchao/sparsity/README.md:56-60 — Documents Int8DynamicActivationInt8WeightConfig(layout=SemiSparseLayout()) usage that no longer works after this PR. The "int8 dynamic quant + 2:4 sparsity" section (lines 50-60) should be removed or updated.

  2. torchao/sparsity/README.md:66from torchao.dtypes import SemiSparseLayout in the "2:4 sparsity" code example. This import is now dead.

  3. test/core/test_config.py:67 — Contains a commented-out line # Int8DynamicActivationInt8WeightConfig(layout=SemiSparseLayout()). Minor, but this is now a stale comment referencing deleted code.

These are not blocking — fine to address in a follow-up.


@jerryzh168
Copy link
Copy Markdown
Contributor Author

@claude review again

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

Claude finished @jerryzh168's task in 2m 34s —— View job


Review of PR #4148: Delete deprecated SemiSparseLayout and related code

  • Gather context and read PR diff
  • Review all changed files
  • Check for leftover references to deleted code
  • Post review feedback

Overall: Clean deletion PR. The removal of SemiSparseLayout, SemiSparseAQTTensorImpl, int8_dynamic_activation_int8_semi_sparse_weight, and associated tests/benchmarks is thorough. All the __init__.py exports, dispatch registrations, and import sites have been properly cleaned up. The previous review's nit about the stale comment in test/core/test_config.py:67 has been addressed. LGTM with the following nits.

Nit: Leftover references

  1. torchao/_models/sam/eval_combo.py:347-354 — The helper functions attn_only, mlp_lin1_only, and mlp_lin2_only are now dead code. They were only used by the deleted int8_dynamic_quant_sparse branch. Consider removing them.
    Fix this →

  2. torchao/_models/sam/benchmark.sh:14-15 — Still references --compress int8_dynamic_quant_sparse which will now hit the assert compress is None and fail at runtime.
    Fix this →

  3. torchao/_models/sam/results.csv:6 — Contains a result row for int8_dynamic_quant_sparse that references the removed mode. Minor, but stale data.

  4. torchao/sparsity/README.md:17-19 — Still mentions composing int8 dynamic quantization with 2:4 sparsity in the benchmark narrative (lines 17-19, 29), referencing the configuration that was removed. The benchmark table row at line 29 also shows the composite int8 dynamic quant + 2:4 sparsity result that is no longer reproducible via the codebase.

These are non-blocking — fine to address in a follow-up if preferred.


View job

@jerryzh168
Copy link
Copy Markdown
Contributor Author

@claude review again

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

Claude finished @jerryzh168's task in 1m 49s —— View job


Review of PR #4148: Delete deprecated SemiSparseLayout and related code

  • Gather context and read PR diff
  • Review all changed files
  • Check for leftover references to deleted code
  • Post review feedback

Overall: Clean deletion PR. All previous review nits have been addressed — there are zero remaining references to SemiSparseLayout, SemiSparseAQTTensorImpl, int8_dynamic_activation_int8_semi_sparse_weight, or int8_dynamic_quant_sparse in the codebase. The dead helper functions (attn_only, mlp_lin1_only, mlp_lin2_only) have been removed, the benchmark script and results CSV are cleaned up, and the README has been properly updated. LGTM with two very minor nits.

Nit: Duplicate mlp_only definition in eval_combo.py

torchao/_models/sam/eval_combo.py:346-347 defines mlp_only at the outer function scope, and then eval_combo.py:355-356 redefines it identically inside the sparse_mlp_only branch. The outer definition on line 346 is now dead code — it was previously used by the removed int8_dynamic_quant_sparse branch. Either remove the outer definition or remove the inner one (and rely on the outer one).

Nit: Blank line in results.csv

torchao/_models/sam/results.csv:6 — The removed int8_dynamic_quant_sparse row was replaced with a blank line, leaving an empty line between the sparse row and the int4_weight_only_sparse row. Could clean this up by removing the blank line.

Both are non-blocking.


View job

@jerryzh168
Copy link
Copy Markdown
Contributor Author

@claude review again

@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

Claude finished @jerryzh168's task in 1m 20s —— View job


Review of PR #4148: Delete deprecated SemiSparseLayout and related code

  • Gather context and read PR diff
  • Review all changed files
  • Check for leftover references to deleted code
  • Post review feedback

Overall: Clean deletion PR. All previous review nits have been addressed. Zero remaining references to SemiSparseLayout, SemiSparseAQTTensorImpl, int8_dynamic_activation_int8_semi_sparse_weight, int8_dynamic_quant_sparse, or semi_sparse_layout in the codebase. The duplicate mlp_only definition has been removed, the blank line in results.csv is gone, the README has been updated with corrected benchmark numbers, and the dead helper functions have been cleaned up. LGTM — no issues remaining.

Pre-existing nit (not introduced by this PR)

torchao/_models/sam/benchmark.sh:10-13 — The comments "2:4 sparsity (all)" and "2:4 sparsity (mlp only)" appear swapped relative to their --compress flags (sparse_mlp_only vs sparse). This existed before this PR, so not blocking.


View job

@jerryzh168 jerryzh168 changed the base branch from gh/jerryzh168/64/base to main March 24, 2026 19:51
@jerryzh168 jerryzh168 merged commit 436b2e9 into main Mar 24, 2026
32 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: not user facing Use this tag if you don't want this PR to show up in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants