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

[luci-interpreter] Support RoPE operation #14104

Merged
merged 4 commits into from
Oct 1, 2024

Conversation

ys44kim
Copy link
Contributor

@ys44kim ys44kim commented Sep 26, 2024

This commit supports RoPE kernel in luci-interpreter

ONE-DCO-1.0-Signed-off-by: youngsik kim [email protected]

draft : #13978
issue : #13972

seanshpark
seanshpark previously approved these changes Sep 27, 2024
Copy link
Contributor

@seanshpark seanshpark left a comment

Choose a reason for hiding this comment

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

LGTM

@ys44kim
Copy link
Contributor Author

ys44kim commented Sep 30, 2024

@jinevening
please review it.

{
for (int32_t i2 = 0; i2 < i2_n; ++i2)
{
for (int32_t i3 = 0; i3 < i3_n / 2; ++i3_n)
Copy link
Contributor

Choose a reason for hiding this comment

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

i3_n is const int32_t. How is this updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i made a mistake while changing the variable name.
i will update it. thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

CI just passed this code, which means that this code is not compiled. You should add RoPE to compiler/luci-interpreter/pal/linux/KernelsToBuild.lst.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i prepared another PR.
as you comment, it would be better to make it together.
i will make PR including kernelsToBuild.lst and loader.

@ys44kim ys44kim force-pushed the rope/luci-interpreter branch 2 times, most recently from e3950c1 to 6726d24 Compare September 30, 2024 02:36
@ys44kim ys44kim changed the title [luci-interpreter] Add RoPE Kernel [luci-interpreter] Support RoPE operation Sep 30, 2024
@ys44kim ys44kim force-pushed the rope/luci-interpreter branch 2 times, most recently from d43184f to b221cb9 Compare September 30, 2024 04:18
This commit supports RoPE for luci-interpreter

ONE-DCO-1.0-Signed-off-by: youngsik kim [email protected]
void RoPE::configure()
{
LUCI_INTERPRETER_CHECK(input()->shape().num_dims() == 4);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from seanshpark,
the head size (dim(3)) of input and the size of sin/cos should be the same.
so, it is necessary to check the condition here.

This Commit adds check condition for sin/cos table

ONE-DCO-1.0-Signed-off-by: youngsik kim <[email protected]>
This commit adds test case to check sin/cos table

ONE-DCO-1.0-Signed-off-by: youngsik kim <[email protected]>
This commits add test case to check rope mode

ONE-DCO-1.0-Signed-off-by: youngsik kim [email protected]
Copy link
Contributor

@jinevening jinevening left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@seanshpark seanshpark left a comment

Choose a reason for hiding this comment

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

LGTM

@seanshpark seanshpark merged commit 6535079 into Samsung:master Oct 1, 2024
8 checks passed
@ys44kim ys44kim deleted the rope/luci-interpreter branch October 2, 2024 00:49
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.

3 participants