If the positions "x_coor" and "y_coor" should be swapped in Line 466 and 468 det3d/models/necks/rpn_transformer?

[Castiel-Lee]

Those two lines seem flawless.
But coincidently, I changed the range of PC, which made H unequal to W. I encountered this:

Afte debugging, in get_multi_scale_feature, center_pos sometimes fell out of the range of feat. After checking backwards, I found some elements of y_coor > W exist. in Line 468.
After some experiments I tried, this problem can be fixed by swaping x_coord and y_coord.

[edwardzhou130]

Thanks for reporting this error. I think the position is correct. I generate the index in the 2D matrix in this way:

centerformer/det3d/models/necks/rpn_transformer.py

Lines 247 to 250 in 5a949b8

	feat_id = (
	neighbor_coords[:, :, :, 1] * (W // (2**i))
	+ neighbor_coords[:, :, :, 0]
	) # pixel id [B, 500, k]

Here neighbor_coords[:, :, :, 1] comes from the y_coord and neighbor_coords[:, :, :, 0] comes from the x_coord.

However, the bug is probably this:

centerformer/det3d/models/necks/rpn_transformer.py

Lines 244 to 246 in 5a949b8

	neighbor_coords = torch.clamp(
	neighbor_coords, min=0, max=H // (2**i) - 1
	) # prevent out of bound

The coordinates should be clamped separately using their own size rather than just assuming H = W.

I don't have a machine at hand to test. Can you check if changing this can fix your error?

[Castiel-Lee]

Hello,

I notice that, the bug reporting can disappear through x_coor and y_coor being clamped within [0, H] and [0, W]. But it could probably leave the fact behind that x_coor = order // W and y_coor = order % W. I did a small test:

According to the picture, the revised version seems correctly make x_coor = 7 and y_coor =4.

Could you think this over and recheck the implementation when you are available? Of course, if "transposing" or some other purposes exist, I would apologize for my misunderstanding and bothering you.

[edwardzhou130]

Oh, I see. So the x_coor and y_coor here do not mean the row and col indexes of this value in the 2D matrix (like [7,4]). Conversely, it means the indexes on the height dimension (y_coor) and width dimension (x_coor), which are common to describe the coordinates of a pixel in an image.

I will recheck the code just in case there are some other bugs.

[Castiel-Lee]

Oh, I see. So the x_coor and y_coor here do not mean the row and col indexes of this value in the 2D matrix (like [7,4]). Conversely, it means the indexes on the height dimension (y_coor) and width dimension (x_coor), which are common to describe the coordinates of a pixel in an image.

I will recheck the code just in case there are some other bugs.

Since height dimension (y_coor) and width dimension (x_coor), in Line 469, it should be torch.stack([y_coor, x_coor], dim=2), right?

[edwardzhou130]

The order of the x_coor and y_coor variables does not matter in this case, as long as you remember the correct dimension that each variable represents. The deformable attention layer also requires storing the reference point position in the same way.

[Castiel-Lee]

Oh, I see. I will go through it again and recheck it. Thank you so much for the clarification.

[edwardzhou130]

Great! If you have any further questions or concerns, feel free to reopen the issue.