[Feature] Add visibility prediction head #2417

Billccx · 2023-05-31T07:13:13Z

Motivation

Enable top-down methods to predict keypoints visibility

Modification

add a new head vis_head in hybrid_heads (mmpose/mmpose/models/heads/hybrid_heads/vis_head.py)
add a new TTA function flip_visibility (mmpose/mmpose/models/utils/tta.py)
add a new key transformed_keypoints_visible in label_mapping_table (mmpose/mmpose/datasets/transforms/formatting.py)
support keypoints_visible in GenerateTarget (mmpose/mmpose/datasets/transforms/common_transforms.py)
~~add a new loss function BCEWithLogitsLoss (mmpose/mmpose/models/losses/classification_loss.py)~~
refine BCELoss module: add parameter with_logits to decide whether to use BCEWithLogitsLoss (mmpose/mmpose/models/losses/classification_loss.py)
add unittest for vis_head (mmpose/tests/test_models/test_heads/test_hybrid_heads/test_vis_head.py)

BC-breaking (Optional)

Use cases (Optional)

We can use vis_head to wrap a keypoints prediction head, getting the keypoints prediction results with keypoints visibility.

For example, in config file:

head=dict(
  type='VisPredictHead',
  loss=dict(type='BCELoss', use_target_weight=False, with_logits=True),
  use_sigmoid=False,
  pose_cfg=dict(
    type='HeatmapHead',
    in_channels=32,
    out_channels=17,
    deconv_out_channels=None,
    loss=dict(type='KeypointMSELoss', use_target_weight=True),
    decoder=codec
  )
)

Checklist

Before PR:

I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
New functionalities are covered by complete unit tests. If not, please add more unit tests to ensure correctness.
The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

CLA has been signed and all committers have signed the CLA in this PR.

Ben-Louis · 2023-05-31T09:38:14Z

mmpose/models/heads/hybrid_heads/vis_head.py

+        for index in range(len(pose_pred_instances)):
+            pose_pred_instances[index].keypoint_visibility = batch_vis_np[
+                index]


Why has a loop been utilized here? BTW, the visibility representation in 'pred_instances' is denoted by 'keypoint_scores'

Why has a loop been utilized here? BTW, the visibility representation in 'pred_instances' is denoted by 'keypoint_scores'

Thanks for your advise. I will rewrite this part soon, sorry for the bad code.

I'm sorry, my tone may have led to a misunderstanding. I was just unclear whether 'pose_pred_instances' is a tuple or InstanceData, so I wanted to ask if there is a possibility of not using a loop. Take it easy~

I'm sorry, my tone may have led to a misunderstanding. I was just unclear whether 'pose_pred_instances' is a tuple or InstanceData, so I wanted to ask if there is a possibility of not using a loop. Take it easy~

Got it! Sorry for the late reply.

From my perspective, 'pose_pred_instances' is an 'InstanceList'?

mmpose/mmpose/models/heads/base_head.py

Lines 78 to 81 in 2c4a60e

preds = [

InstanceData(keypoints=keypoints, keypoint_scores=scores)

for keypoints, scores in zip(batch_keypoints, batch_scores)

]

mmpose/mmpose/models/heads/heatmap_heads/heatmap_head.py

Lines 235 to 241 in 2c4a60e

Returns:

Union[InstanceList | Tuple[InstanceList | PixelDataList]]: If

``test_cfg['output_heatmap']==True``, return both pose and heatmap

prediction; otherwise only return the pose prediction.

The pose prediction is a list of ``InstanceData``, each contains

the following fields:

Besides that, I have refined the code. If I still get something wrong, please let me know. Thanks.

codecov · 2023-06-06T12:38:43Z

Codecov Report

Patch coverage: 86.00% and project coverage change: +0.12 🎉

Comparison is base (0c02149) 80.67% compared to head (6612dd1) 80.79%.

❗ Current head 6612dd1 differs from pull request most recent head bd18401. Consider uploading reports for the commit bd18401 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           dev-1.x    #2417      +/-   ##
===========================================
+ Coverage    80.67%   80.79%   +0.12%     
===========================================
  Files          250      251       +1     
  Lines        15112    15208      +96     
  Branches      2618     2632      +14     
===========================================
+ Hits         12191    12288      +97     
+ Misses        2267     2257      -10     
- Partials       654      663       +9

Flag	Coverage Δ
unittests	`80.79% <86.00%> (+0.12%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
mmpose/datasets/transforms/formatting.py	`78.40% <0.00%> (-1.83%)`	⬇️
mmpose/datasets/transforms/common_transforms.py	`86.38% <60.00%> (+0.47%)`	⬆️
mmpose/models/heads/hybrid_heads/vis_head.py	`88.09% <88.09%> (ø)`
mmpose/models/heads/__init__.py	`100.00% <100.00%> (ø)`
mmpose/models/heads/hybrid_heads/__init__.py	`100.00% <100.00%> (ø)`
mmpose/models/losses/classification_loss.py	`87.20% <100.00%> (+6.97%)`	⬆️
mmpose/models/utils/tta.py	`82.85% <100.00%> (+1.03%)`	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Ben-Louis · 2023-06-13T06:08:07Z

mmpose/models/heads/hybrid_heads/vis_head.py

+        if not self.use_sigmoid:
+            batch_vis = torch.sigmoid(batch_vis)


Why is sigmoid applied to batch_vis when use_sigmoid is False?

I think the value of visibility prediction should be between 0 and 1? If use_sigmoid is False, there won't be sigmoid in vis_forward.

If the sigmoid function is always applied to the head outputs, what is the role of the use_sigmoid argument?

Why is sigmoid applied to batch_vis when use_sigmoid is False?

@Ben-Louis When use_sigmoid is True, a nn.Sigmoid() will be in the vis_head, namely, the batch_vis is between 0~1. Otherwise, we need to do sigmoid manually when use_sigmoid is False.

Using nn.Sigmoid() and BCELoss separately is not officially recommended by Pytorch, a better way is to use BCEWithLogitsLoss and do sigmoid manually when a 0-1 score is needed. So I suggest him to set a use_sigmoid to keep the flexity.

@Tau-J, thanks for the clarification. This can indeed be puzzling, perhaps we could add some comments for clarity? Also, focusing on a single recommended practice could be beneficial. BTW, in MMDetection, this parameter is generally integrated in loss modules like mmdet.CrossEntropyLoss. We might consider refining the BCELoss module with this parameter, rather than introducing a completely new loss module.

mm-assistant bot assigned Ben-Louis May 31, 2023

Ben-Louis reviewed May 31, 2023

View reviewed changes

Billccx force-pushed the Billccx/refactor_contributing_doc branch from 4794d2a to 582f26c Compare June 11, 2023 07:31

Billccx requested a review from Ben-Louis June 13, 2023 05:58

Ben-Louis reviewed Jun 13, 2023

View reviewed changes

Billccx added 4 commits June 17, 2023 10:19

add vis_head

d3ba1b9

add acc_vis

7ec801b

refine vis_head

88f33c1

refine BCELoss and add test_vis_head

1ee410a

Billccx force-pushed the Billccx/refactor_contributing_doc branch from 582f26c to 1ee410a Compare June 17, 2023 17:47

Billccx requested review from Tau-J and Ben-Louis June 17, 2023 18:06

fix ut

bd18401

Tau-J approved these changes Jun 19, 2023

View reviewed changes

Tau-J merged commit 1340c3a into open-mmlab:dev-1.x Jun 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Feature] Add visibility prediction head #2417

[Feature] Add visibility prediction head #2417

Uh oh!

Billccx commented May 31, 2023 •

edited

Loading

Uh oh!

Ben-Louis May 31, 2023

Uh oh!

Billccx May 31, 2023

Uh oh!

Ben-Louis May 31, 2023

Uh oh!

Billccx Jun 11, 2023

Uh oh!

codecov bot commented Jun 6, 2023 •

edited

Loading

Uh oh!

Ben-Louis Jun 13, 2023

Uh oh!

Billccx Jun 13, 2023

Uh oh!

Ben-Louis Jun 13, 2023

Uh oh!

Tau-J Jun 13, 2023

Uh oh!

Tau-J Jun 13, 2023

Uh oh!

Ben-Louis Jun 13, 2023

Uh oh!

Uh oh!

	preds = [
	InstanceData(keypoints=keypoints, keypoint_scores=scores)
	for keypoints, scores in zip(batch_keypoints, batch_scores)
	]

	Returns:
	Union[InstanceList \| Tuple[InstanceList \| PixelDataList]]: If
	``test_cfg['output_heatmap']==True``, return both pose and heatmap
	prediction; otherwise only return the pose prediction.

	The pose prediction is a list of ``InstanceData``, each contains
	the following fields:

		if not self.use_sigmoid:
		batch_vis = torch.sigmoid(batch_vis)

[Feature] Add visibility prediction head #2417

[Feature] Add visibility prediction head #2417

Uh oh!

Conversation

Billccx commented May 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modification

BC-breaking (Optional)

Use cases (Optional)

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Billccx commented May 31, 2023 •

edited

Loading

codecov bot commented Jun 6, 2023 •

edited

Loading