search

sefibk / KernelGAN

Other
337 stars 77 forks source link

Copy patches instead of view #35

Closed alexander-rakhlin closed 4 years ago

alexander-rakhlin commented 4 years ago

Every time you add noise to crop_im you change original self.input_image https://github.com/sefibk/KernelGAN/blob/7218a37f0933519513aac97f40b8a72d3676eec9/data.py#L44

should change crop_im = self.input_image[top:top + size, left:left + size, :] to crop_im = self.input_image[top:top + size, left:left + size, :].copy()

sefibk commented 4 years ago
  1. Are you sure? did you verify this is right?
  2. Wouldn't your suggestion create multiple copies of the input_image?
alexander-rakhlin commented 4 years ago

Yes, the difference is view vs copy. Slicing shares data with the original object https://scipy-cookbook.readthedocs.io/items/ViewsVsCopies.html

Another thing you may implement here is to move self.input_image to GPU at initialization. This should improve speed (but not by much in my case) crop_im = self.input_image_cuda[top:top + size, left:left + size, :].data.clone()

sefibk commented 4 years ago

There are definitely a LOT of things to optimize! I am only taking care of serious bugs currently, I am no longer in the field of this work... If you fixed the bug and would like to open a PR I would appreciate the contribution

sefibk commented 4 years ago

Thanks for the PR. I will verify performance is not hurt on the entire dataset and will merge. It may take me a while to get to it