Skip to content

Conversation

@icemiliang
Copy link

@icemiliang icemiliang commented Jun 6, 2019

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
Copy link
Owner

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
Copy link
Author

icemiliang commented Jun 9, 2019

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.

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.

2 participants