-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[Feature] Group-Free-3D head #539
Conversation
Arguments: | ||
seed_features: (batch_size, feature_dim, num_seed) Pytorch tensor | ||
Returns: | ||
logits: (batch_size, 1, num_seed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
descriptions
|
||
def forward(self, xyz, features, sample_inds): | ||
""" | ||
Args: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modify the docstrings and also others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some functions seem to be borrowed from somewhere else? Please refine their docstrings
return new_xyz, new_features, sample_inds | ||
|
||
|
||
class PositionEmbeddingLearned(nn.Module): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we put positional embedding into the registry like those in mmdet/mmcv?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid the specific positional embedding module is not so general that other algorithm would use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put it into the registry as it is general enough and may be used in other transformer-based 3d detector.
Tensor: (B, C, M) the sampled features. | ||
Tensor: (B, M) the given index. | ||
""" | ||
xyz_flipped = xyz.transpose(1, 2).contiguous() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use permute(0, 2, 1)
to replace transpose().contiguous()
# (num_candidate, ) with values in 0,1,...,gt_num-1 | ||
assignment = query_points_instance_label | ||
# set background points to the last gt bbox as original code | ||
assignment[assignment < 0] = gt_num - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a little bit weird and might cause a bug
jinhui seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Codecov Report
@@ Coverage Diff @@
## master #539 +/- ##
==========================================
- Coverage 51.02% 50.19% -0.84%
==========================================
Files 204 208 +4
Lines 15395 15909 +514
Branches 2492 2555 +63
==========================================
+ Hits 7856 7986 +130
- Misses 7021 7402 +381
- Partials 518 521 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Need to resolve conflicts @hjin2902 |
Group-Free-3D head
* Transformer decoder/GroupFree3DMultiheadAttention
* GroupFree3DBBoxCoder
* loss
* get boxes
* test case/config