fangchangma / self-supervised-depth-completion

ICRA 2019 "Self-supervised Sparse-to-Dense: Self-supervised Depth Completion from LiDAR and Monocular Camera"
MIT License
623 stars 135 forks source link

change gray image code from g to y #25

Open icemiliang opened 5 years ago

icemiliang commented 5 years ago

Before: 'g' for gray, 'rgb' for rgb.

Problem: They both have a 'g' and args.use_g is checked before args.use_rgb in kittki_loader. A redundant gray image might be created when the user only chooses rgb.

After: 'y' for gray, 'rgb' for rgb.

fangchangma commented 5 years ago

Thanks for the pull request.

To gain a better understanding of the PR: where exactly in the code does the "overriding" create a problem (in the DepthCompletionNet definition, or in the self-supervised framework)? By design, args.use_g and args.use_rgb are not exclusive.

icemiliang commented 5 years ago

In main.py line 63, because 'rgb' also has a 'g', when choosing 'rgb' as the input, use_g will be set to true when the input is either an rgb image or a gray image.

args.use_rgb = ('rgb' in args.input) or args.use_pose
args.use_d = 'd' in args.input
args.use_g = 'g' in args.input

However, in kitti_loader.py line 189, "if not args.use_g", this condition cannot differentiate rgb and gray and will be skipped when either rgb or g were selected. Thus, the function will not only return rgb even if the user has chosen rgb alone as the input.

def handle_gray(rgb, args):
    if rgb is None:
        return None, None
    if not args.use_g:
        return rgb, None
    else:
        img = np.array(Image.fromarray(rgb).convert('L'))
        img = np.expand_dims(img,-1)
        if not args.use_rgb:
            rgb_ret = None
        else:
            rgb_ret = rgb
        return rgb_ret, img

I checked DepthCompletionNet and it shouldn't be a problem there since 'rgb' in modality is checked before 'g' in modality, so the network and the framework should be OK (I have updated the pull request message). Only a redundant gray image might be created in dataloader when user chooses 'rgb' as the input. But I still think the code for gray image should be changed because by definition user_g will be set to true whenever 'rgb' is selected and it also brings confusion and vulnerability.