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

ndimage: add grid_mode option to zoom #4401

Merged
merged 9 commits into from
Dec 21, 2020

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Dec 3, 2020

blocked by #4400 (This PR builds on that one. Only the final "ENH: implement grid_mode parameter for zoom" commit here is new)

This PR implements zooming in a manner consistent with scikit-image's resize and OpenCV's cv2.resize. See further explanation in the corresponding SciPy PR scipy/scipy#13095 (to be released in SciPy 1.6).

Basically the issue relates to whether coordinate (0, 0) is at the center or corner of a pixel. In the kernel, this translates into computing coordinates via:
c = zoom * (in_coord + 0.5) - 0.5 (for grid_mode=True)
vs.
c = zoom * in_coord (for grid_mode=False)

@grlee77 grlee77 changed the title add grid_mode option to ndimage.zoom ndimage: add grid_mode option to zoom Dec 4, 2020
@kmaehashi kmaehashi added cat:enhancement Improvements to existing features prio:medium st:blocked-by-another-pr Blocked by another pull-request labels Dec 4, 2020
The changes here are consistent with a large number of fixes introduced for SciPy 1.6
Specifically this PR includes order=1 and order=0 fixes from
SciPy's gh-12767 and gh-12776
STYLE: f-strings in _spline_prefilter_core.py

pep 8 fixes
All ndimage test cases now verified to pass when using SciPy 1.6.0 pre-release code
@takagi
Copy link
Member

takagi commented Dec 17, 2020

pfnCI, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit e23dd31, target branch master) succeeded!

@takagi
Copy link
Member

takagi commented Dec 18, 2020

As well as #4400, would you mark cupy.experimental if grid_mode is supplied by the user. LGTM except for that.

@grlee77
Copy link
Contributor Author

grlee77 commented Dec 18, 2020

Thanks for reviewing @takagi, I have added the experimental marker here and in #4400

@takagi
Copy link
Member

takagi commented Dec 21, 2020

pfnCI, test this please.

@takagi
Copy link
Member

takagi commented Dec 21, 2020

Thanks! I'll mrege this with following #4400 .

@chainer-ci
Copy link
Member

Jenkins CI test (for commit ddc31ab, target branch master) succeeded!

@takagi takagi removed the st:blocked-by-another-pr Blocked by another pull-request label Dec 21, 2020
@takagi takagi added this to the v9.0.0b1 milestone Dec 21, 2020
@takagi takagi merged commit ef6d007 into cupy:master Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:enhancement Improvements to existing features prio:medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants