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

Fix Get Valid Counts when the number of boxes is zero #7229

Merged
merged 1 commit into from
Jan 8, 2021

Conversation

mbrookhart
Copy link
Contributor

Thanks @trevor-m and @anijain2305 for finding this issue. Sometimes a model may propose 0 boxes, and the code had a place that ended up assigning uninitialized memory to valid_counts, which could cause memory access issues in NMS.

The diff is kind of ugly, but this PR simply wraps the exclusive scan in this:

    with ib.if_scope(num_anchors > 0):
        # Old code
    with ib.else_scope():
        with ib.new_scope():
            bx = te.thread_axis("blockIdx.x")
            ib.scope_attr(bx, "thread_extent", batch_size)
            with ib.if_scope(bx < batch_size):
                valid_count[bx] = 0

Which runs the algorithm if there are boxes to check, and otherwise assigns valid_count to 0.

Unfortunately, since the issue is randomly uninitialized memory getting assigned to valid_count, I haven't been able to write a unit test that consistently fails with the old code, so this just includes the change.

@mbrookhart
Copy link
Contributor Author

cc @masahi

@masahi
Copy link
Member

masahi commented Jan 8, 2021

This is not important, but I'd suggest swapping the outer most then block and else block (with ib.if_scope(num_anchors == 0):), since the else block is so far apart and hard to read. Alternatively, if you wrap the content of then block into a function (like nms_inner_loop), it might make the diff smaller (just my guess).

You don't have to do that change, maybe I can do that for my scan PR.

@trevor-m @anijain2305 Does this solve your issue?

@masahi masahi merged commit 4e8cc4f into apache:main Jan 8, 2021
@masahi
Copy link
Member

masahi commented Jan 8, 2021

Thanks @mbrookhart

@mbrookhart mbrookhart deleted the fix_get_valid_counts_for_zero_boxes branch January 8, 2021 16:58
tkonolige pushed a commit to tkonolige/incubator-tvm that referenced this pull request Jan 11, 2021
masahi pushed a commit to masahi/tvm that referenced this pull request Jan 14, 2021
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Jan 20, 2021
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jan 21, 2021
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants