Skip to content

[Tracing] Autowrap methods by name #1390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

kylesayrs
Copy link
Collaborator

@kylesayrs kylesayrs commented Apr 27, 2025

Purpose

  • Reduce model support burden by allowing users to skip untraceable methods using string matching
  • This would commonly be used for skipping _update_causal_mask, which is the default

TODO

  • Update documentation to reflect new usage

Prerequisites

Changes

  • Update ignore documentation to reflect how ignore is now used to ignore methods, not modules
  • Implement add_autowrap_methods which finds module methods to autowrap
  • Add tracing_ignore, which defaults to _update_causal_mask

Testing

  • Add tests for add_autowrap_methods

Follow ups

  • Catch tracing failures and suggest that users ignore methods in the stack trace
  • In the future, support autowrapping functions or code

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>
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Copy link

👋 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.

@kylesayrs kylesayrs changed the base branch from main to kylesayrs/skip-non-ancestors April 27, 2025 17:59
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs kylesayrs added the ready When a PR is ready for review label Apr 27, 2025
@kylesayrs kylesayrs removed the ready When a PR is ready for review label Apr 27, 2025
@kylesayrs kylesayrs marked this pull request as draft April 27, 2025 18:20
Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Base automatically changed from kylesayrs/skip-non-ancestors to main April 29, 2025 16:05
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant