Skip to content

[Tracing][Testing] Add tracing tests#1335

Merged
kylesayrs merged 21 commits intomainfrom
kylesayrs/tracing-testing
May 5, 2025
Merged

[Tracing][Testing] Add tracing tests#1335
kylesayrs merged 21 commits intomainfrom
kylesayrs/tracing-testing

Conversation

@kylesayrs
Copy link
Copy Markdown
Collaborator

@kylesayrs kylesayrs commented Apr 8, 2025

Purpose

  • Add regression testing to model tracing beyond example tests
    • These tests complete in ~1 min and can be run at a quicker cadence than example tests
    • These can also be used to test tracing capabilities beyond those in the examples, for example tracing into linear layers

Prerequisites

Changes

  • Fix function signature of peoples speech dataset
  • Add trust_remote_code argument to debugger
  • Add tests/llmcompressor/transformers/tracing/models.py
    • I did not include phi3 because it's a very difficult model to work with programmatically. I will revisit once the major tracing improvements have landed

Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2025

👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review.

Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed.

Copy link
Copy Markdown
Collaborator

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

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

good stuff!

@kylesayrs kylesayrs changed the title [Tracing] [Testing] Add tracing tests [Tracing][Testing] Add tracing tests Apr 8, 2025
@kylesayrs kylesayrs added the ready When a PR is ready for review label Apr 8, 2025
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs
Copy link
Copy Markdown
Collaborator Author

This is waiting on getting dedicated HF tokens to add to transformers tests

Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Copy link
Copy Markdown
Collaborator

@andy-neuma andy-neuma left a comment

Choose a reason for hiding this comment

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

let me know if the token works

@kylesayrs
Copy link
Copy Markdown
Collaborator Author

kylesayrs commented Apr 28, 2025

FYI this PR is waiting on gated access to the meta-llama/Llama-3.2-11B-Vision-Instruct and Llama4 models

EDIT: works now, thanks @andy-neuma !

kylesayrs added a commit that referenced this pull request Apr 29, 2025
## Purpose ##
* Reduce model support burden by skipping any modules which are not call
graph ancestors of the sequential targets
* Rather than requiring the user to specify a list of ignored modules,
only trace what is necessary to disjointly execute sequential targets
* In the future, the ignore field will be used to skip untraceable
function/method names
* This change does not change functionality because all ignored modules
are already non-ancestors of sequential targets

## Changes ##
* Remove `ignore` modules requirement (all ignored modules are already
non-ancestors of sequential targets)
* Implement `get_sequential_ancestors` which returns all ancestors of
the sequential targets
* Modify tracer to skip anything that is not a sequential ancestor or
has offloaded modules
* The two sets rarely overlap, and when they do, the module is skipped
for safety and the user is warned

## Testing ##
* Added tests for `get_sequential_ancestors`
* #1335

## Follow ups ##
* #1390

---------

Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
dsikka added a commit that referenced this pull request Apr 30, 2025
## Purpose ##
* When #1389 landed, modules being skipped by ignore were no longer
being skipped. However, this requires that the sequential targets list
be correct. Mllama defaults to targeting vision layers, and hence the
vision tower was being traced, leading to errors.

```python3
_no_split_modules = [
    "MllamaVisionEncoderLayer",
    "MllamaCrossAttentionDecoderLayer",
    "MllamaSelfAttentionDecoderLayer",
]
```

## Changes ##
* Only target text decoder layers, not vision decoder layers

## Testing ##
* #1335 passes

Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Co-authored-by: Dipika Sikka <dipikasikka1@gmail.com>
Copy link
Copy Markdown
Collaborator

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

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

nice!

Copy link
Copy Markdown
Collaborator

@rahul-tuli rahul-tuli left a comment

Choose a reason for hiding this comment

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

Lgtm!

@kylesayrs kylesayrs enabled auto-merge (squash) May 5, 2025 19:39
@kylesayrs kylesayrs merged commit ffa570c into main May 5, 2025
8 checks passed
@kylesayrs kylesayrs deleted the kylesayrs/tracing-testing branch May 5, 2025 20:15
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 review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants