Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operator support for F.hardswish #92

Closed
shaharelys opened this issue Mar 27, 2024 · 3 comments 路 Fixed by #100
Closed

Operator support for F.hardswish #92

shaharelys opened this issue Mar 27, 2024 · 3 comments 路 Fixed by #100
Labels
enhancement New feature or request help wanted Extra attention is needed operators

Comments

@shaharelys
Copy link
Contributor

shaharelys commented Mar 27, 2024

馃殌 Feature

Implement HardSwish activation function.

Motivation

Relatively easy activation function implementation as a good first issue as nikitaved suggested under #64

Pitch

Add HardSwish (x * ReLU6(x + 3) / 6) leveraging existing ReLU6 support.

cc @apaz-cli

@shaharelys shaharelys added enhancement New feature or request help wanted Extra attention is needed labels Mar 27, 2024
@shaharelys shaharelys changed the title Operator support for F.hardswish Operator support for F.hardswish Mar 27, 2024
@shaharelys
Copy link
Contributor Author

Hey @nikitaved (@carmocca you are more than welcome to assist me as well!), based on our discussions under #64 and your helpful suggestions, I've started working on implementing F.hardswish as a good first issue.

Here's the code snippet I came up with (I used relu6 as this is how hard-swish was initially introduced in Searching for MobileNetV3 but could achieve the same with clamp. Please lmk if there is any preference). Could you please take a quick look and suggest the next steps?

@torchsymbol(torch.nn.functional.hardswish, is_method=False)
def hardswish(a: TensorProxy, /, inplace: bool = False) -> TensorLike:
    utils.check(not inplace, lambda: f"hardswish only supports inplace=False", exception_type=NotImplementedError)
    return a * relu6(a + 3) / 6

For relu6, which is the most related function, I see a few related lines in thunder/executors/torchex.py, thunder/tests/opinfos.py. Should I follow the existing pattern there, or are there other adjustments I should consider?

Looking forward to your feedback and further guidance!

@nikitaved
Copy link
Contributor

Hey, @shaharelys ! Yes, this looks great! And yes, we have to insert these things which are present there for relu6. It is very important to implement tests as well, as otherwise we can not guarantee that everything is connected properly. I am currently on holiday, but I will be back next Tuesday to help you out and expand on why we do things certain way (as per your last post in the one hot issue) :) Cheers!

@mruberry
Copy link
Collaborator

mruberry commented Apr 1, 2024

This has been addressed now that #100 is merged! Thanks @shaharelys!

@mruberry mruberry closed this as completed Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed operators
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants