Skip to content

RoIHeads: remove low-scoring boxes statically#4

Merged
sprt merged 1 commit intojysohn23:tpu_compatfrom
sprt:tpu_compat
Dec 11, 2019
Merged

RoIHeads: remove low-scoring boxes statically#4
sprt merged 1 commit intojysohn23:tpu_compatfrom
sprt:tpu_compat

Conversation

@sprt
Copy link
Copy Markdown
Collaborator

@sprt sprt commented Dec 11, 2019

One of the postprocessing step of RoIHeads is to filter low-scoring (in terms of confidence level) boxes from the output. This commit makes that operation static for TPUs.

For now, it's needed to make it static manually as the experimental support for dynamic nonzero/masked_select would raise the following issue in this case:

RuntimeError: Internal: From /job:tpu_worker/replica:0/task:0:
RET_CHECK failure (learning/brain/google/xla/tpu_execute.cc:1066) size >= 0
	 [[{{node XRTExecute}}]]

One of the postprocessing step of RoIHeads is to filter low-scoring (in
terms of confidence level) boxes from the output. This commit makes that
operation static for TPUs.

For now, it's needed to make it static manually as the experimental
support for dynamic nonzero/masked_select would raise the following
issue in this case:

RuntimeError: Internal: From /job:tpu_worker/replica:0/task:0:
RET_CHECK failure (learning/brain/google/xla/tpu_execute.cc:1066) size >= 0
	 [[{{node XRTExecute}}]]
@taylanbil
Copy link
Copy Markdown

@sprt Could you elaborate on how this makes it static? what does making static "manually" mean?

@sprt
Copy link
Copy Markdown
Collaborator Author

sprt commented Dec 11, 2019

@taylanbil I mean "manually" in the sense that I'm not relying on the experimental support for dynamic shapes with nonzero/masked_select (because it seems buggy atm, see exception), and rather making the shapes dynamic static by hand using torch.where(). Before this change, the shape of inds was dynamic since the number of boxes which score are above the self.score_thresh is variable across images.

@taylanbil
Copy link
Copy Markdown

ah I see. Thanks.

@sprt sprt merged commit 9f7946a into jysohn23:tpu_compat Dec 11, 2019
sprt added a commit that referenced this pull request Dec 14, 2019
pytorch#1019 introduced a change that removes empty boxes in
postprocessing on the basis that, previously, empty boxes would actually
reach this postprocessing step, and that the model could therefore
output empty boxes (even though NMS would've most likely filtered them
out). It's essentially a safety check that'd be seldom needed.

However, that filtering causes dynamicity on the TPU (because the number
of empty boxes, if any, would be unknown). And in any case, we recently
introduced a change in PR #4 that purposefully pads the boxes tensor
with empty boxes, to avoid dynamicity. Therefore there's no point trying
to remove boxes that we use as padding, we'll just filter those out of
the output on the CPU.
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.

3 participants