Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change gray image code from g to y #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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