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/service] Infer dynamic shape for pad #13780

Merged
merged 13 commits into from
Aug 29, 2024

Conversation

icodo98
Copy link
Contributor

@icodo98 icodo98 commented Aug 27, 2024

This infers dynmic shape for pad.
If input shape is unknown, output shape is also unknown.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]


Draft : #13735

This infers dynmic shape for pad.
If input shape is unknown, output shape is also unknown.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee <[email protected]>
ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
added _NEG test case for Pad

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
This migrates pad Op's shape inference rule to sinf::Algorithm.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
@icodo98 icodo98 added PR/ready for review It is ready to review. Please review it. SSAFY labels Aug 27, 2024
@icodo98 icodo98 requested review from zetwhite and shs-park August 27, 2024 06:03
@icodo98 icodo98 changed the title [luci] infer dynamic shape for pad [luci/service] infer dynamic shape for pad Aug 27, 2024
@shs-park shs-park requested a review from a team August 27, 2024 06:32
@shs-park
Copy link
Contributor

Please run ./nnas format and check the code format.
We are following clang formatting.
Please note - #12311

fix code format with suggestions.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]

Co-authored-by: SaeHie Park <[email protected]>
@icodo98 icodo98 requested a review from seanshpark August 28, 2024 02:12
move use_paddings funtion to new .h file

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
@icodo98 icodo98 force-pushed the dynamic_infer_pad branch from b4f9d00 to 8aa6138 Compare August 28, 2024 02:59
@zetwhite
Copy link
Contributor

(minor things)

Could you retitle this PR? [luci/service] infer .. -> [luci/service] Infer ..
There is a convention in ONE repo - that PR title starts with a capital letter.

@icodo98 icodo98 changed the title [luci/service] infer dynamic shape for pad [luci/service] Infer dynamic shape for pad Aug 28, 2024
rename file to HelperPads.h

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
Explicitly express namespace sinf int CiclePad.cpp.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
add to-do for non-const case

Co-authored-by: SaeHie Park <[email protected]>
use graph() in test code.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
…to dynamic_infer_pad

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
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!

Copy link
Contributor

@zetwhite zetwhite 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

@llFreetimell llFreetimell left a comment

Choose a reason for hiding this comment

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

LGTM
// this PR can be merged without my approval even after this review :)

@seanshpark
Copy link
Contributor

waiting for other two more review approvals :)

@zetwhite zetwhite requested a review from Hanjin-Choi August 29, 2024 05:16
@Hanjin-Choi Hanjin-Choi removed their request for review August 29, 2024 05:20
@zetwhite
Copy link
Contributor

@Hanjin-Choi Could you review this PR?
We usually get all approvals of whom participate (leave any kinds of comment) in reviewing.

@Hanjin-Choi Hanjin-Choi self-requested a review August 29, 2024 05:21
Copy link
Contributor

@Hanjin-Choi Hanjin-Choi 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

@shs-park shs-park left a comment

Choose a reason for hiding this comment

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

Thank you!
LGTM
=)

@seanshpark seanshpark merged commit 61b6a11 into Samsung:master Aug 29, 2024
10 checks passed
@icodo98 icodo98 deleted the dynamic_infer_pad branch October 2, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it. SSAFY
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants