Implement reprojection logic

[alexandreflusin]

Before, the plot_2d method of the Camera class was used to plot both the 2d video and the 2d part of the 3d video without making being able to re-project the 3d points calculated by triangulation. Because of that, it was plotting the points from 2d pose estimation for both videos. The function was changed so that it can take both 2d and 3d points as argument and then distinguish them based on their dimension. If the argument has 3d points, the method projects them and then plots them. They are transposed when passed in the cv2 functions because those use the (column, row) format

[jasper-tms]

Do we need the double brackets [[img_id]]? If not, change to single

[sibocw]

yes i have the same question

[sibocw]

I think this is clearer, but check for syntax and typo etc.

if points is None:
    points = self.points2d

if points.shape[-1] == 3:
    # points are given as 3d coords
    points3d = points[img_id]  # (n_joints, 3)
    points2d = self.project(points[np.newaxis, :, :]).squeeze()  # project works only in batches
elif points.shape[-1] == 2:
    points2d = points
else:
    raise ...

# Then use points2d instead of points

[sibocw]

General comment: Variable names are a bit confusing. There's points2d, then there's points which can be either in 2D or 3D. This makes the code hard to understand and maintain. I suggest the following naming it to a local variable pose2d after the projection (if applicable).

[alexandreflusin]

When I don't use the double brackets I get the following error

File "/home/df3d-student/Desktop/repos/PyBundleAdjustment/pyba/Camera.py", line 161, in plot_2d
points = self.project(points).squeeze()
^^^^^^^^^^^^^^^^^^^^
File "/home/df3d-student/Desktop/repos/PyBundleAdjustment/pyba/Camera.py", line 188, in project
assert points3d.ndim == 3 and points3d.shape[2] == 3
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I use the double brackets to preserve the dimensions of points

[Dominic-DallOsto]

Being able to choose whether to plot the original or reprojected 2D points sounds useful to help with understanding how df3d works!

I just think that the structure of the code is a bit cleaner if plot_2d stays the way it is (plotting a list of 2D points) and the code that calls this function is responsible for also calling project when we have 3D points to plot.

I checked through the code and this functionality is actually already available in the CameraNetwork.plot_2d function via the points parameter. You can see an example in this notebook in the 51 and 53 cells. But it isn't as straightforward as I'd hoped to just reuse it.

Confusingly there are a whole bunch of different plot_2d functions scattered around the codebase, with different functionality

• PyBA Camera.plot_2d plots points for that one camera
  • here we can pass in a different set of 2D points if we want
• PyBA CameraNetwork.plot_2d plots points for all the cameras in the network
  • here we can choose whether to use reprojected points or not
• DF3D core.plot_2d plots points for one camera
  • here we can choose whether to use manually corrected points or not
  • this directly uses Camera.plot_2d not CameraNetwork.plot_2d

I was trying to think of a way of restructuring this so things are more straightforward, but it seems a bit tangled and hard to easily separate without breaking old code (which might be worth doing to simplify things). If you have any suggestions for better ways of structuring this, I'd be happy to hear them!

So I think the easist option (while still keeping things clear) would be to create a new Camera.plot_3d function that plots 3d points, which calls project then plot_2d. I think this is just a bit less confusing than having plot_2d sometimes plot 2d points, somtimes 3d points, and sometimes the default 2d points it has from when it was initialised?

• then CameraNetwork.plot_2d could make use of plot_3d if the reprojection option is requested.
• core.plot_2d could have an option added whether to use reprojected points
• and a corresponding option added to the CLI

Let me know whether that makes sense and what you think!

[jasper-tms]

Yes, overall that makes sense to me!

So I think the easist option (while still keeping things clear) would be to create a new Camera.plot_3d function that plots 3d points, which calls project then plot_2d.

To me a function named Camera.plot_3d should create a 3d plot, not create a 2d plot from 3d points. So I think a name like Camera.plot_reprojections might make more sense to me. But yes, having that be a function like that which just calls project and then plot_2d seems overall like a good structure to me.

I think that df3d's --plot-3d argument should always plot reprojected points, not the original 2d predictions. We discussed this a bit in lab, and thought that because there's also the --plot-2d argument, we don't need to allow users to create a 3d video that has the raw 2d points – the reprojected points would be better to show in the 2d images shown above the 3d renderings. If people want to see the raw 2d points, they can ask for --plot-2d. Does that seem reasonable to you Dom?

@alexandreflusin we're happy to take things from here, or leave it to you if you have time and interest. What would you prefer?

[Dominic-DallOsto]

To me a function named Camera.plot_3d should create a 3d plot, not create a 2d plot from 3d points. So I think a name like Camera.plot_reprojections might make more sense to me.

Ahh yep, gotcha!

I think that df3d's --plot-3d argument should always plot reprojected points, not the original 2d predictions. We discussed this a bit in lab, and thought that because there's also the --plot-2d argument, we don't need to allow users to create a 3d video that has the raw 2d points – the reprojected points would be better to show in the 2d images shown above the 3d renderings. If people want to see the raw 2d points, they can ask for --plot-2d. Does that seem reasonable to you Dom?

Yeah, that also makes sense! And keeps the CLI less complex which is good I think haha.

[alexandreflusin]

I ll be happy to try and make these changes!