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

[cker] Introduce cker for avgpool #14086

Merged
merged 3 commits into from
Oct 1, 2024
Merged

Conversation

icodo98
Copy link
Contributor

@icodo98 icodo98 commented Sep 25, 2024

This PR introduce cker for average pooling operation.

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

@icodo98 icodo98 added the PR/ready for review It is ready to review. Please review it. label Sep 25, 2024
@icodo98 icodo98 requested a review from a team September 25, 2024 02:27
@icodo98
Copy link
Contributor Author

icodo98 commented Sep 25, 2024

related : #13829
draft: #14059

This commit add chekr for average pooling

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
compute/cker/src/train/AvgPool.test.cc Outdated Show resolved Hide resolved
compute/cker/src/train/AvgPool.test.cc Outdated Show resolved Hide resolved
compute/cker/include/cker/train/operation/AvgPool.h Outdated Show resolved Hide resolved
@zetwhite zetwhite self-requested a review September 26, 2024 00:46
out_mat.cwiseMin(params.float_activation_min).cwiseMax(params.float_activation_max);
}

inline void AvgPool2DGrad(const PoolParams &params, const Shape &incoming_shape,
Copy link
Contributor

@zetwhite zetwhite Sep 26, 2024

Choose a reason for hiding this comment

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

Could you add the backward formula for avgpool, here? #11248

AvgPoolOpVerifier<float> verifier(op_param, in, out);

/**
* input(index) : output(arg-count):
Copy link
Contributor

@zetwhite zetwhite Sep 26, 2024

Choose a reason for hiding this comment

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

(optional)

Ah, If you followed the maxpool2d notation, maxpool2d expressed the index - because one output of maxpool2d is about index.

For avergepool2d, index isn't important, I guess you don't need to mark the index here.
If you have another intention about index plz let me know.

AvgPoolOpVerifier<float> verifier(op_param, in, out);

/**
* input(index) : output(arg-count):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* input(index) : output(arg-count):
* input : output(arg-count):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, If you followed the maxpool2d notation

I did follow the maxpool2d notation. I'll remove it.

use existing forwarding function.
update notaion.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
@icodo98
Copy link
Contributor Author

icodo98 commented Sep 29, 2024

I update it!
PTAL
=)

* limitations under the License.
*/

#ifndef __NNFW_CKER_TRAIN_OPERATION_AVGPOOL_H__
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#ifndef __NNFW_CKER_TRAIN_OPERATION_AVGPOOL_H__
#ifndef __NNFW_CKER_TRAIN_OPERATION_AVERAGEPOOL_H__

Same to the following two occurrences

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

ragmani
ragmani previously approved these changes Sep 30, 2024
Copy link
Contributor

@ragmani ragmani left a comment

Choose a reason for hiding this comment

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

LGTM

zetwhite
zetwhite previously approved these changes Sep 30, 2024
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

This commit updates header guard.

ONE-DCO-1.0-Signed-off-by: JuYoung Lee [email protected]
@icodo98 icodo98 dismissed stale reviews from zetwhite and ragmani via 0cacca9 September 30, 2024 08:44
@glistening glistening merged commit 5d8026f into Samsung:master Oct 1, 2024
9 checks passed
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants