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 PETR transforms and box handling to work with old checkpoints #2919

Open
wants to merge 2 commits into
base: dev-1.x
Choose a base branch
from

Conversation

carlinds
Copy link

@carlinds carlinds commented Mar 5, 2024

Motivation

Current version of the PETR-project does not produce the reported metrics (see #2510 for more details). The current state of the project has the following issues:

  • Incorrect rotation and scaling of the camera pose when doing 3D transforms.
  • Incorrect normalizing (and denormalizing) of bounding boxes. Current normalizing is not compatible with the checkpoint in the repo, effectively switching length and width, causing large scale errors.

Modification

This PR fixes:

  • The 3D transforms to work correctly with versions >v1.0.0 (i.e after the coordinate system refactoring).
  • The normalizing (and denormalizing) of boxes to work with the old checkpoint.

Results from testing the PR on nuScenes validation set (with the old checkpoint):

mAP: 0.3829
mATE: 0.7376
mASE: 0.2702
mAOE: 0.4803
mAVE: 0.8703
mAAE: 0.2040
NDS: 0.4352

Log from the test: 20240305_103838.log

@CLAassistant
Copy link

CLAassistant commented Mar 5, 2024

CLA assistant check
All committers have signed the CLA.

@bradyz
Copy link

bradyz commented Mar 22, 2024

Wow great catch.
Have you tried training with these changes?

[scale_ratio, 0, 0, 0],
[0, scale_ratio, 0, 0],
[0, 0, scale_ratio, 0],
[0, 0, 0, 1],
])

rot_mat_inv = torch.inverse(rot_mat)
scale_mat_inv = np.linalg.inverse(scale_mat)
Copy link

@TillBeemelmanns TillBeemelmanns Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np.linalg.inverse() does not exist AttributeError: module 'numpy.linalg' has no attribute 'inverse'
Should be np.linalg.inv()

rot_mat = np.array([[rot_cos, rot_sin, 0, 0],
[-rot_sin, rot_cos, 0, 0], [0, 0, 1, 0],
[0, 0, 0, 1]])
rot_mat_inv = np.linalg.inverse(rot_mat)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@TillBeemelmanns
Copy link

TillBeemelmanns commented Apr 2, 2024

I trained PETR with the current code and with the suggestions in this PR. Both versions end up at the same metrics with only marginal differences:

NuScenes metric/pred_instances_3d_NuScenes/NDS: 0.4321
NuScenes metric/pred_instances_3d_NuScenes/mAP: 0.3766 

vs.

NuScenes metric/pred_instances_3d_NuScenes/NDS: 0.4353
NuScenes metric/pred_instances_3d_NuScenes/mAP: 0.3771

I would not merge this PR, but rather provide new weights for the current setup.

@bradyz
Copy link

bradyz commented Apr 2, 2024

here's a little weight conversion script -
let me know if i missed something, i only did a quick qualitative check

import torch

ckpt = torch.load('weights/petr.pth')

keys = [
    'pts_bbox_head.reg_branches.0.4.weight',
    'pts_bbox_head.reg_branches.1.4.weight',
    'pts_bbox_head.reg_branches.2.4.weight',
    'pts_bbox_head.reg_branches.3.4.weight',
    'pts_bbox_head.reg_branches.4.4.weight',
    'pts_bbox_head.reg_branches.5.4.weight',
]

for k in keys:
    orig = ckpt['state_dict'][k]

    # weights are (10, 256)
    modified = orig.clone()
    modified[2:3] = orig[3:4]
    modified[3:4] = orig[2:3]

    ckpt['state_dict'][k] = modified

torch.save(ckpt, 'weights/petr_fixed.pth')

@carlinds
Copy link
Author

carlinds commented Apr 3, 2024

I trained PETR with the current code and with the suggestions in this PR. Both versions end up at the same metrics with only marginal differences:

NuScenes metric/pred_instances_3d_NuScenes/NDS: 0.4321
NuScenes metric/pred_instances_3d_NuScenes/mAP: 0.3766 

vs.

NuScenes metric/pred_instances_3d_NuScenes/NDS: 0.4353
NuScenes metric/pred_instances_3d_NuScenes/mAP: 0.3771

I would not merge this PR, but rather provide new weights for the current setup.

I trained PETR with the current code and with the suggestions in this PR. Both versions end up at the same metrics with only marginal differences:

NuScenes metric/pred_instances_3d_NuScenes/NDS: 0.4321
NuScenes metric/pred_instances_3d_NuScenes/mAP: 0.3766 

vs.

NuScenes metric/pred_instances_3d_NuScenes/NDS: 0.4353
NuScenes metric/pred_instances_3d_NuScenes/mAP: 0.3771

I would not merge this PR, but rather provide new weights for the current setup.

Providing new weights would also suffice. However, I would still suggest keeping some of the fixes in this request (such as the incorrect rotation, misleading naming and unnecessary conversion between torch and numpy).

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.

4 participants