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

[onert/train] support AveragePool2D for training #13829

Open
jyoungyun opened this issue Aug 29, 2024 · 9 comments
Open

[onert/train] support AveragePool2D for training #13829

jyoungyun opened this issue Aug 29, 2024 · 9 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@jyoungyun
Copy link
Contributor

What

Let's support AveragePool2D for training.

Why?

The InceptionV3 model from TensorFlow applications has AveragePool2D operation. However, onert does not support training this operation. PoolLayer is already implemented and only supports MaxPool2D. This issue can expand the pool type to support AveragePool2D.

@jyoungyun jyoungyun added the help wanted Extra attention is needed label Aug 29, 2024
@zetwhite zetwhite added the good first issue Good for newcomers label Aug 29, 2024
@icodo98
Copy link
Contributor

icodo98 commented Sep 24, 2024

I made a draft for this issue. #14059
PTAL 😊

@icodo98
Copy link
Contributor

icodo98 commented Sep 24, 2024

Here is sample model and result.
averagePool.zip

image

@icodo98
Copy link
Contributor

icodo98 commented Sep 27, 2024

from #14086 (comment)

while replacing code to use existing forwarding function, I've got a question.

for MaxPool2D in training, it's able to forward output from padding area.

/**
* input_with_padding: expected_output:
*
* 4 8 0 0 0
* 9 2 -(forward)-> 0 9 0
* 0 0 0
*/

for average pool, I think the output will be

    /**
     * input_with_padding:             expected_output:
     *
     *    4   8                              0    0    0
     *    9   2            -(forward)->      0  5.75   0
     *                                       0    0    0
     */

But with current forwarding AveragePool function, above test case fails.

// Divide the output by the actual number of elements being averaged over
assert(out_count.minCoeff() > 0);
out_mat.array().rowwise() /= out_count.transpose().array();

assert(out_count >0) prevent output induced from padded layer.

So, is our policy to not induce output from padded area? or is it bug?

/cc @ragmani @zetwhite

@ragmani
Copy link
Contributor

ragmani commented Sep 27, 2024

So, is our policy to not induce output from padded area? or is it bug?

It's the former. There is no case that circle model has paddings like the above test. And onert only allows padding type as "valid" and "same" for Pool ops now. The two padding types never induce Pool ops to have paddings like the above test.

@zetwhite
Copy link
Contributor

zetwhite commented Sep 27, 2024

And onert only allows padding type as "valid" and "same" for Pool ops now

Ah, I've had a different understanding. I thought that "explicit" type is also possible for onert Pool ops. Because while generating kernel for pool operator, KernelGenerator allows explicit pool params.

  • KernelGenerator.cc, calculate padding

    const auto padding = ir::calculatePadding(node.param().padding, ifm_shape.asFeature(),
    ofm_shape.asFeature(), stride, kw, kh);

  • calculatePadding() it allows explicit padding type

    const ExplicitPadding calculatePadding(const Padding &padding, const FeatureShape &ifm_shape,
    const FeatureShape &ofm_shape, const Stride &stride,
    uint32_t kw, uint32_t kh, uint32_t dwf, uint32_t dhf)
    {
    if (padding.type == PaddingType::EXPLICIT)
    {
    return padding.param;
    }
    else if (padding.type == PaddingType::SAME)
    {
    return samePadding(ifm_shape, ofm_shape, stride, kw, kh, dwf, dhf);
    }
    else if (padding.type == PaddingType::VALID)
    {
    return validPadding();
    }
    else
    {
    throw std::runtime_error{"Cannot handle padding type"};
    }
    }


There is no case that circle model has paddings like the above test.

Maybe Does it mean that the frontend does not allow explicit type?

@ragmani
Copy link
Contributor

ragmani commented Sep 27, 2024

Maybe Does it mean that the frontend does not allow explicit type?

I guess that training also does not allow explicit type for pool ops. As far as I know, explicit type may be allowed only in the case of pad op now.

@icodo98
Copy link
Contributor

icodo98 commented Sep 28, 2024

As far as I know, explicit type may be allowed only in the case of pad op now.

Then, I'll remove that test case from PR.

@icodo98
Copy link
Contributor

icodo98 commented Oct 14, 2024

Even after #14149 , InceptionV3 model is not supported for training because Concat operation is not trainable in One yet.
Can I open new issue for this?

@shs-park
Copy link
Contributor

Sure, just make a link to this issue as related =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants