ucbdrive / hd3

Code for Hierarchical Discrete Distribution Decomposition for Match Density Estimation (CVPR 2019)
BSD 3-Clause "New" or "Revised" License
204 stars 31 forks source link

Extra minus #27

Closed avdmitry closed 4 years ago

avdmitry commented 4 years ago

Hello,

Thank you for sharing the code.

Looks like, there shouldn't be minus before read_pfm_file(), please, check.

https://github.com/ucbdrive/hd3/blob/c62cb3183f7d9c1b5f3534db59b3dbfef3d21c8f/utils/flowlib.py#L260

yzcjtr commented 4 years ago

Hi, I just checked it over the FlyingThings3D subset. The loaded disparity from the pfm file is all positive. So to my mind, this minus is necessary. You can validate it by yourself as well.

avdmitry commented 4 years ago

@yzcjtr, in general, training&validation of stereo model on synthetic data didn't work proper for me. Removing this minus fixed the issue.

yzcjtr commented 4 years ago

The stereo model in the model zoo is trained with the minus kept. I didn't notice any abnormal behavior in terms of the validation/test results. Also, from the definition of the function, the returned numpy array should contain all positive values if the mode is 'stereo'. Removing the minus sign contradicts this definition.

Can you give more details on why it didn't work properly for you?

avdmitry commented 4 years ago

That's strange. The issue can be easily reproduced by running inference.py with pretrained model hd3sc_things-57947496.pth.

yzcjtr commented 4 years ago

Thanks for the details. Can you make sure you are using the FlyingThings3D subset instead of the original dataset? Also, can you post your full script/command here so I can check it?

hofingermarkus commented 4 years ago

Since I was currently debugging a stereo case I can share which values I saw, if that helps you: For the FlyingThings3D subset, the left disparities are negative and the right disparities are positive when I load them with directly with read_pfm_file

For Stereo the sign is flipped twice in the hd3 dataloader, first by read_disp (called by read_gen), second by fl.disp2flow https://github.com/ucbdrive/hd3/blob/141398985ab1059cc2b5768b13175540329d57d5/data/hd3data.py#L38-L40

read_disp flips the sign here: https://github.com/ucbdrive/hd3/blob/c62cb3183f7d9c1b5f3534db59b3dbfef3d21c8f/utils/flowlib.py#L260 disp2flow of the utils/flowlibs.py flips again (however, the disp2flow in hd3_ops doesn't) https://github.com/ucbdrive/hd3/blob/141398985ab1059cc2b5768b13175540329d57d5/utils/flowlib.py#L272

So for the left disparities (as in the FlyingThings3D_stereo_val.txt) the values that reach hd3net.py are negative (at least in my debug sesssion)

yzcjtr commented 4 years ago

Hi @hofingermarkus, thanks for your comments. Negative values fed to hd3net.py is exactly what we want since we convert the left disparity to optical flow (see https://github.com/ucbdrive/hd3/blob/141398985ab1059cc2b5768b13175540329d57d5/data/hd3data.py#L10). I checked again through the code and found no bugs. Please keep in mind when we talk about disparity we assume it to be always positive without direction. After the conversion to optical flow, it will possibly get flipped signs for we have the concept of direction now.

As for the problem pointed out by @avdmitry, I ran my script with the model he mentioned on the validation set of FlyingThings3D subset. The EPE is 1.58. The visualization is good. Everything looks normal to me. I guess this is because he didn't use the FlyingThings3D subset I mentioned in the readme file.

I'm closing this issue for the bug doesn't exist actually.