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

[New Op] Added dropout unary op #12474

Merged
merged 26 commits into from
Sep 16, 2024
Merged

[New Op] Added dropout unary op #12474

merged 26 commits into from
Sep 16, 2024

Conversation

dmakoviichuk-tt
Copy link
Contributor

@dmakoviichuk-tt dmakoviichuk-tt commented Sep 10, 2024

Ticket

None

Problem description

I've found that we have lkk kernel but no unary dropout.

What's changed

Added unary op: dropout.
Added usage example.

Unfortunately reproducibility is low. I was able to get same results twice only if I reset device between calls.
I implemented using_dropout.py example but didn't add tests because I cannot get the same result twice.
Feedback to the kernel team was provided.
Also std is pretty high but I can use it.
Comparison vs pytorch:

TTNN:
Expected probability: 0.2
Mean: 0.1978125
Standard Deviation: 0.1085284200969958
PYTORCH:
Expected probability: 0.2
Mean: 0.199869384765625
Standard Deviation: 0.002019949620664387

https://github.com/tenstorrent/tt-metal/actions/runs/10874629279

Checklist

  • Post commit CI passes
  • Blackhole Post commit (if applicable)
  • Model regression CI testing passes (if applicable)
  • New/Existing tests provide coverage for changes

@@ -278,6 +290,39 @@ Tensor Softplus::invoke(
optional_output_tensor);
}

Tensor Dropout::invoke(
Copy link
Member

Choose a reason for hiding this comment

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

Wdyt about extracting this to its own .hpp/cpp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't want to do it right now. It is implemented the same way as any other op and uses its infra.

Copy link
Member

@ayerofieiev-tt ayerofieiev-tt left a comment

Choose a reason for hiding this comment

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

Overall LGTM, but format change makes it hard to distinguish actual change from everything else. I think it is best to revert style changes.

@dmakoviichuk-tt dmakoviichuk-tt changed the title [WIP] Added dropout unary op [New Op] Added dropout unary op Sep 11, 2024
@dmakoviichuk-tt dmakoviichuk-tt marked this pull request as ready for review September 11, 2024 17:33
return detail::unary_impl(
DefaultQueueId,
input,
{UnaryWithParam{UnaryOpType::DROPOUT, {static_cast<float>(seed), probability, scale}}},
Copy link
Contributor

Choose a reason for hiding this comment

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

why seed needs to be float?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @ayerofieiev-tt ahahah :-D

ttnn/examples/usage/using_dropout.py Outdated Show resolved Hide resolved
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.

6 participants