Skip to content

Bilinear mode in topi::nn::upsampling not working #1692

Description

@ke1337

To repro, use following changes in test_topi_upsampling.py:

--- a/topi/tests/python/test_topi_upsampling.py
+++ b/topi/tests/python/test_topi_upsampling.py
@@ -22,7 +22,7 @@ def verify_upsampling(batch, in_channel, in_height, in_width, scale, layout='NCH
         raise NotImplementedError(
             'Layout not supported {} '.format(layout))

-    B = topi.nn.upsampling(A, scale, layout=layout)
+    B = topi.nn.upsampling(A, scale, layout=layout, method='BILINEAR')

     b_np = topi.testing.upsampling_python(a_np, scale, layout)

Activity

  1. masahi commented on Sep 9, 2018

    @masahi
    Member
  2. srkreddy1238 commented on Sep 9, 2018

    @srkreddy1238
    Contributor

    @KeDengMS

    NCH layout ???
    Can you conform the error log ?

  3. ke1337 commented on Sep 10, 2018

    @ke1337
    Author

    Yes NCHW layout, but I believe it exists in NHWC as well. The error is from a crash in dereferencing nullptr in here. The problem is that shape is a Mul node with two IntImm as input from the python API, and as_const_int returns nullptr for non-const node.
    It seems const folding should be applied, and moreover the bilinear interpolation code path needs to be tested.

  4. srkreddy1238 commented on Sep 11, 2018

    @srkreddy1238
    Contributor

    @KeDengMS a bit confused.

    Is the issue about "Layout Not Supported" or "Crash " ?

    Can you share complete log or the sample code?

  5. ke1337 commented on Sep 11, 2018

    @ke1337
    Author

    I think it's nothing about Layout, but more for BILINEAR mode. The repro is to change the python test to use BILINEAR mode: B = topi.nn.upsampling(A, scale, layout=layout, method='BILINEAR'), then you'll find the crash.

  6. masahi commented on Sep 12, 2018

    @masahi
    Member

    But the log says NotImplementedError('Layout not supported {} '.format(layout)). What is the "layout" parameter you passed in? It should be "NCHW" or "NHWC".

  7. ke1337 commented on Sep 12, 2018

    @ke1337
    Author

    The repro I show above is from git diff, not the log. The testcase tests both NCHW and NHWC

  8. masahi commented on Sep 12, 2018

    @masahi
    Member

    @KeDengMS I can repro your crash. The fix is to add util.simplify to https://github.com/dmlc/tvm/blob/master/topi/python/topi/nn/upsampling.py#L34
    This is equivalent to const folding you mentioned.

    I can send a PR to fix this and update the upsampling test.

  9. masahi commented on Sep 12, 2018

    @masahi
    Member

    @srkreddy1238 I made a branch to fix this issue here. But some bilinear upsampling tests are failing. Can you have a look?

  10. added a commit that references this issue on Sep 12, 2018
  11. srkreddy1238 commented on Sep 12, 2018

    @srkreddy1238
    Contributor

    Thanks @KeDengMS for bringing up the issue and thanks @masahi for finding the root cause and fix.
    I have added the final piece (atol=1e-5 allowance) at #1708 for your review comment.

  12. added a commit that references this issue on Sep 12, 2018
  13. ke1337 commented on Sep 12, 2018

    @ke1337
    Author

    Thanks for the fix!

  14. added a commit that references this issue on Dec 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions