Conversation
phanicode
force-pushed
the
SP-2017-photo-opt
branch
from
670cb2f to
c53fedd
Compare
| deriv, deriv_scale, img = item | ||
| else: | ||
| deriv, img = item | ||
| deriv_scale = 1.0 |
There was a problem hiding this comment.
I would add an inline comment here reminding that deriv_scale is unity so the vals calculation just multiplies by a constant
| patch_h, patch_w = patch_data.shape | ||
|
|
||
| # 1. Equivalent to get_overlapping_region for Y | ||
| y_start = max(0, -y0) |
There was a problem hiding this comment.
Please add a docstring clarifying what the input parameters represent. I understand it is trying to find the overlap between mod_img and patch_data, but it's unclear which is the "reference" vs "desired" size.
| assert(np.all(np.isfinite(um.patch))) | ||
| # print 'Adding umod', um, 'with counts', counts, 'to mod', mod.shape | ||
| (um * counts).addTo(mod) | ||
| # (um * counts).addTo(mod) |
There was a problem hiding this comment.
We can either remove this (since it's commented out), or keep it in as reference for which original function we have substituted/replaced. Which do you think makes more sense?
gtorrini
left a comment
gtorrini
left a comment
There was a problem hiding this comment.
Left a few comments/requests for clarification. Please ping me when it is ready again, and I will approve once I have successfully visualized/understood what's going on in the fast add improvement. Thank you for the work!
3 participants
Description of changes
Patch.mul seems to allocate np.zeros, this does not need to happen for our purposes , and the scale can be refactored and multiplied downstream.