ccjoechou commented on pull request #9235:
URL: https://github.com/apache/tvm/pull/9235#issuecomment-943494906


   Hi Wuwei, (or any other who can help?)
   
   I have spent lots time on this pooling issue, which I believe it is a 
fundamental-concept-related issue.
   Similar to what we have implemented for the ConvInferCorrectLayout() 
function in the tvm/src/relay/op/nn/convention.h file – for instance, it keeps 
the conv op’s layouts, if they are provided.
   
   Let me use the nn.max_pool2d op as an example.
   If in a given (input) IR graph, it is “explicitly” described that a 
nn.max_pool2d op needs to use the layout == “NHWC”.
   
   
     *   E.g.: y = relay.nn.max_pool2d(y, pool_size=(2, 2), layout="NHWC")
   
   Plus, there is either:
   
   
     *   Either, “no” transforms.ConvertLayout() is given with a desired layout 
for nn.max_pool2d op, or
     *   A transforms.ConvertLayout() is given with a desired layout for 
"nn.max_pool2d": ["NHWC"] – i.e., to tell TVM.relay that the nn.max_pool2d op 
must stick with its “NHWC” data layout format (for input and for output)
   
   In the above conditions, shouldn’t TVM.relay keep the nn.max_pool2d op’s 
input, data, output layouts as “NHWC”?
   That is when nn.max_pool2d op’s param->layout != “” (assuming it is a legal 
layout format), we should use:
   
     *   InferCorrectLayoutOutput({use the original, given params->layout}, 
{use the original, given params->layout}, Attrs(params))
   
   If this fundamental concept stands, then, the PR-#9235’s change to 
pooling.cc file should be (kind of) correct !
   
   If you are in agreement with this fundamental concept, then, I can make 
further changes to the following “test suites” …
   
   
     *   
Tvm/tests/python/contrib/test_vitis_ai/test_vitis_ai_runtime_cpu_part.py – by 
change its golden result to have 18 tvm ops (instead of 7, originally)
        *   Performance Note: this can affect the visit ai team’s run-time 
performance because, now, 11 more ops will not be executed on the visit ai 
accelerator (based on their byoc comments)
        *   It is possible that we may need to provide an additional pool 
param->layout == ?? so that it can force
     *   tests/python/relay/test_pass_alter_op_layout.py – by making similar 
changes as I did to the tvm/tests/python/relay/test_pass_convert_op_layout.py 
test suite.
   
   … Please advice so that I can continue and then resolve this PR.
   
   Thanks a lot !
   
   
     *   Joe
   
   
   From: Joe Chou
   Sent: Monday, October 11, 2021 12:05 PM
   To: apache/tvm ***@***.***>; apache/tvm ***@***.***>
   Cc: Author ***@***.***>
   Subject: RE: [EXT] Re: [apache/tvm] [bug] pooling convert layout bug in 
pooling.cc and in test_pass_conve… (see also issue #8848) (#9235)
   
   Yes and I am trying to reproduce them locally in order to find resolutions.
   
   
     *   Joe
   
   From: Wuwei Lin ***@***.******@***.***>>
   Sent: Monday, October 11, 2021 12:00 PM
   To: apache/tvm ***@***.******@***.***>>
   Cc: Joe Chou ***@***.******@***.***>>; Author ***@***.******@***.***>>
   Subject: [EXT] Re: [apache/tvm] [bug] pooling convert layout bug in 
pooling.cc and in test_pass_conve… (see also issue #8848) (#9235)
   
   External Email
   ________________________________
   
   There are some failing test cases on CI that relate to this change
   
   —
   You are receiving this because you authored the thread.
   Reply to this email directly, view it on 
GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apache_tvm_pull_9235-23issuecomment-2D940364473&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=84zI-iH2xz28Q_xujSGbjYaq38CgcRgl_vGzCtF6TwQ&m=escaZCs9wQeKyQiDz74nwKHPHsm5mss21WkH6-ffeSk&s=Mdy5MIJStP_P1-hkWJjXslSooRthmG2tR48tsd6o5Tg&e=>,
 or 
unsubscribe<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AM636PADO63ICYSJVE5VHLLUGMXZFANCNFSM5FUWAG4A&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=84zI-iH2xz28Q_xujSGbjYaq38CgcRgl_vGzCtF6TwQ&m=escaZCs9wQeKyQiDz74nwKHPHsm5mss21WkH6-ffeSk&s=GGqpJDj2F6zu0XWc28cFTudrEPooKQUKfAiBQoZlecA&e=>.
   Triage notifications on the go with GitHub Mobile for 
iOS<https://urldefense.proofpoint.com/v2/url?u=https-3A__apps.apple.com_app_apple-2Dstore_id1477376905-3Fct-3Dnotification-2Demail-26mt-3D8-26pt-3D524675&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=84zI-iH2xz28Q_xujSGbjYaq38CgcRgl_vGzCtF6TwQ&m=escaZCs9wQeKyQiDz74nwKHPHsm5mss21WkH6-ffeSk&s=xoFKW0tfvMfEkDFOnw9BnAhKtdJVug-1lHe6Arsxh-c&e=>
 or 
Android<https://urldefense.proofpoint.com/v2/url?u=https-3A__play.google.com_store_apps_details-3Fid-3Dcom.github.android-26referrer-3Dutm-5Fcampaign-253Dnotification-2Demail-2526utm-5Fmedium-253Demail-2526utm-5Fsource-253Dgithub&d=DwMCaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=84zI-iH2xz28Q_xujSGbjYaq38CgcRgl_vGzCtF6TwQ&m=escaZCs9wQeKyQiDz74nwKHPHsm5mss21WkH6-ffeSk&s=W1tjMqF4DUukbs-p5E7Zxx507jXS0S3LEDr_tHLx0Ig&e=>.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to