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.

Sign up for GitHub

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

Jump to bottom

[luci/lang] Add CircleGRU node #12598

Merged
merged 2 commits into from
Mar 13, 2024
Merged

[luci/lang] Add CircleGRU node #12598

merged 2 commits into from
Mar 13, 2024

Conversation

BalyshevArtem
Copy link
Contributor

This pr adds CircleCirGru node in luci/lang

for issue: #12320
from draft: #12319

ONE-DCO-1.0-Signed-off-by: Artem Balyshev [email protected]

@BalyshevArtem BalyshevArtem added the approval: 2 Require at least 2 approvals label Feb 2, 2024
@BalyshevArtem BalyshevArtem mentioned this pull request Feb 2, 2024
Open
18 tasks
@BalyshevArtem
Copy link
Contributor Author

Note: should be rebased after #12597 will be merged

@BalyshevArtem BalyshevArtem force-pushed the circle_gru_luci_lang branch 2 times, most recently from c5d7196 to db131f3 Compare February 21, 2024 07:35
@BalyshevArtem BalyshevArtem changed the title [luci/lang] Add CircleCirGru node [luci/lang] Add CircleGRU node Feb 21, 2024
@BalyshevArtem BalyshevArtem requested review from seanshpark and a team February 21, 2024 07:57
seanshpark
seanshpark previously approved these changes Feb 21, 2024
View reviewed changes
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 thank you!

@seanshpark seanshpark requested a review from a team February 21, 2024 08:22
@BalyshevArtem
Copy link
Contributor Author

@seanshpark, can you please help me to resolve build issue:

[2024-02-21T08:25:01.182Z] /opt/jenkins_agent/workspace/nnfw/master/pr-nncc-release@2/compiler/luci/lang/include/luci/IR/CircleNodes.lst: In member function ‘virtual circle::BuiltinOperator {anonymous}::QueryOpCode::visit(const luci::CircleGRU*)’:

[2024-02-21T08:25:01.182Z] /opt/jenkins_agent/workspace/nnfw/master/pr-nncc-release@2/compiler/luci/partition/src/CircleOpCode.cpp:35:62: error: ‘BuiltinOperator_GRU’ was not declared in this scope

[2024-02-21T08:25:01.182Z]    BuiltinOperator visit(const CIRCLE_CLASS *) final { return BuiltinOperator_##OPCODE; }

[2024-02-21T08:25:01.182Z]                                                               ^~~~~~~~~~~~~~~~

[2024-02-21T08:25:01.182Z] /opt/jenkins_agent/workspace/nnfw/master/pr-nncc-release@2/compiler/luci/lang/include/luci/IR/CircleNodes.lst:55:1: note: in expansion of macro ‘CIRCLE_NODE’

[2024-02-21T08:25:01.182Z]  CIRCLE_NODE(GRU, CircleGRU)

[2024-02-21T08:25:01.182Z]  ^~~~~~~~~~~

[2024-02-21T08:25:01.182Z] [ 15%] Building CXX object compiler/luci/pass/CMakeFiles/luci_pass.dir/src/CircleQuantizer.cpp.o

[2024-02-21T08:25:01.441Z] [ 15%] Building CXX object compiler/luci/lang/CMakeFiles/luci_lang_test.dir/src/Nodes/CircleSoftmax.test.cpp.o

[2024-02-21T08:25:01.699Z] [ 15%] Building CXX object compiler/luci-interpreter/src/kernels/CMakeFiles/luci_interpreter_kernels.dir/Relu6.cpp.o

[2024-02-21T08:25:04.215Z] compiler/luci/partition/CMakeFiles/luci_partition.dir/build.make:62: recipe for target 'compiler/luci/partition/CMakeFiles/luci_partition.dir/src/CircleOpCode.cpp.o' failed

How can I resolve it? Smth with decencies?

@seanshpark
Copy link
Contributor

How can I resolve it? Smth with decencies?

Ah, we need to add mio-circle08 and revise to use libs to use 08
please refer #12177 and it's links for changes.

@seanshpark
Copy link
Contributor

#12597 (comment)

adding 08 version was what I was afraid of... lots of changes are needed.
I think this may take some time as lots of guys here including me are mostly out of office in Feb.

@BalyshevArtem
Copy link
Contributor Author

rebased after merging mio-circe 08
@seanshpark, PTAL :)

@seanshpark
Copy link
Contributor

[2024-03-11T09:17:22.287Z] /opt/jenkins_agent/workspace/nnfw/master/pr-nncc-debug/compiler/luci/partition/src/CircleOpCode.cpp:35:62: error: ‘BuiltinOperator_CIR_GRU’ was not declared in this scope
[2024-03-11T09:17:22.287Z] BuiltinOperator visit(const CIRCLE_CLASS *) final { return BuiltinOperator_##OPCODE; }
[2024-03-11T09:17:22.287Z] /opt/jenkins_agent/workspace/nnfw/master/pr-nncc-debug/compiler/luci/lang/include/luci/IR/CircleNodes.lst:55:1: note: in expansion of macro ‘CIRCLE_NODE’
[2024-03-11T09:17:22.287Z] CIRCLE_NODE(CIR_GRU, CircleCirGru)

jinevening
jinevening reviewed Mar 12, 2024
View reviewed changes
compiler/luci/lang/src/Nodes/CircleCirGru.test.cpp Outdated

#include <gtest/gtest.h>

TEST(CircleCirGruTest, constructor_P)
Copy link
Contributor

Choose a reason for hiding this comment

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

_P is not necessary

Suggested change
TEST(CircleCirGruTest, constructor_P)
TEST(CircleCirGruTest, constructor)

Artem Balyshev added 2 commits March 12, 2024 12:11
[luci/lang] Add CircleCirGru node
9768c31 
This pr adds CircleCirGru node in luci/lang

ONE-DCO-1.0-Signed-off-by: Artem Balyshev <[email protected]>
rename CirGru -> GRU
2a59c7e
seanshpark
seanshpark approved these changes Mar 12, 2024
View reviewed changes
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 thank you!

jinevening
jinevening approved these changes Mar 13, 2024
View reviewed changes
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

@seanshpark seanshpark merged commit 6765b4e into Samsung:master Mar 13, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jinevening jinevening jinevening approved these changes

@seanshpark seanshpark seanshpark approved these changes

Assignees
No one assigned
Labels
approval: 2 Require at least 2 approvals
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BalyshevArtem @seanshpark @jinevening