Skip to content

Conversation

@bnellnm
Copy link
Collaborator

@bnellnm bnellnm commented Jul 12, 2024

This PR implements the first cut of the optimizer decribed here: [RFC] A Graph Optimization System in vLLM using torch.compile

Issue: #6378

@peaceorwell
Copy link

@bnellnm I'd like to ask, I didn't see any pattern matching related code in the PR. Did I miss it?

Choose a reason for hiding this comment

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

I have a small question. The operator has already been registered to torch in torch_binding.cpp above, why does it need to be registered again here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This version of silu_mul_quant includes allocating the output/tmp buffers. The function names here don't really mean anything since we will be going by the torch op name (but probably should be changed to avoid confusion).

@bnellnm
Copy link
Collaborator Author

bnellnm commented Jul 17, 2024

@bnellnm I'd like to ask, I didn't see any pattern matching related code in the PR. Did I miss it?

We don't use pytorch's pattern matching routines. The code in fusion.py searches for and combines fusable operations.

@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 17, 2024
@robertgshaw2-redhat robertgshaw2-redhat removed the ready ONLY add when PR is ready to merge/full CI is needed label Jul 17, 2024
Comment on lines 71 to 77
Copy link
Collaborator Author

@bnellnm bnellnm Jul 23, 2024

Choose a reason for hiding this comment

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

I think this should be

    if isinstance(n, float):
        ty = "float"
    elif isinstance(n, int):
        ty = "int"
    elif n.type is not None:
        ...

so that the proper C++/Python type is returned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the signature of this function should remain as it was before so that we could do a proper job of generating a meta function at some point in the future. The compose stuff doesn't really work for non-unary meta functions.

@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 25, 2024
@bnellnm bnellnm force-pushed the bnell/torch-compile-fusion branch from 9a9af98 to 962d9bb Compare July 26, 2024 02:09
@rkooo567 rkooo567 removed the ready ONLY add when PR is ready to merge/full CI is needed label Jul 29, 2024
@bnellnm bnellnm force-pushed the bnell/torch-compile-fusion branch from e2fa8b8 to 355436f Compare August 28, 2024 19:46
@lnykww
Copy link
Contributor

lnykww commented Oct 14, 2024

@bnellnm is this feature still ongoing?

@bnellnm
Copy link
Collaborator Author

bnellnm commented Oct 14, 2024

lnykww

@bnellnm is this feature still ongoing?

It is, although this PR will probably be superceded by others.

@hmellor
Copy link
Member

hmellor commented Feb 20, 2025

Closing as torch.compile support is now added

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants