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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add sin, cos ops #5

Merged
merged 2 commits into from
May 1, 2023
Merged

Conversation

On-JungWoan
Copy link
Contributor

I added the cos and sin operators. Can you please review?

@converter(torch.Tensor.add, torch.Tensor.__add__, torch.Tensor.__iadd__, torch.Tensor.__radd__, channel_ordering_strategy=ChannelOrderingStrategy.MINIMUM_TRANSPOSITIONS_OR_PYTORCH, autocast=True)
def converter_sin(input, *args, **kwargs):
    def func(input, *args, **kwargs):
        return tf.math.sin(input)
    return func

@converter(torch.Tensor.add, torch.Tensor.__add__, torch.Tensor.__iadd__, torch.Tensor.__radd__, channel_ordering_strategy=ChannelOrderingStrategy.MINIMUM_TRANSPOSITIONS_OR_PYTORCH, autocast=True)
def converter_cos(input, *args, **kwargs):
    def func(input, *args, **kwargs):
        return tf.math.cos(input)
    return func

@AlexanderLutsenko
Copy link
Owner

@converter(torch.Tensor.add, torch.Tensor.add, torch.Tensor.iadd, torch.Tensor.radd, channel_ordering_strategy=ChannelOrderingStrategy.MINIMUM_TRANSPOSITIONS_OR_PYTORCH, autocast=True)

This should be

@converter(torch.sin, channel_ordering_strategy=ChannelOrderingStrategy.MINIMUM_TRANSPOSITIONS)

@On-JungWoan
Copy link
Contributor Author

Ah, I understand it. Any there other problems?

@On-JungWoan
Copy link
Contributor Author

image

@AlexanderLutsenko
Copy link
Owner

AlexanderLutsenko commented May 1, 2023

@On-JungWoan ChannelOrderingStrategy.MINIMUM_TRANSPOSITIONS would be a better fit, just for cleanliness. MINIMUM_TRANSPOSITIONS_OR_PYTORCH is used for special occasions when there's more than one inputs which might require broadcasting (add, mul, etc.). Otherwise, seems legit.

@On-JungWoan
Copy link
Contributor Author

I fixed it!

@AlexanderLutsenko AlexanderLutsenko merged commit 15d904f into AlexanderLutsenko:master May 1, 2023
@On-JungWoan On-JungWoan deleted the pr branch May 1, 2023 17:40
@AlexanderLutsenko
Copy link
Owner

Done

@On-JungWoan
Copy link
Contributor Author

Thank you for merging it😊

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.

None yet

2 participants