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

cupyx.ndimage.measurements: add center_of_mass, histogram, labeled_comprehension #4311

Merged
merged 3 commits into from
Dec 10, 2020

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Nov 21, 2020

This PR adds three functions to cupyx.scipy.ndimage.measurements

  • center_of_mass
  • labeled_comprehension (applies an arbitrary function over each labeled region)
  • histogram (implemented via labeled_comprehension)

One difference in behavior was made to avoid the need for object arrays (in this case, an arrays where each element is an array). This is used for histogram which calls labeled_comprehension using dtype=object so that each element of the object array is the histogram over a particular label. For the CuPy implementation, a list of arrays is returned instead of an object array.

If the return value is a scalar in the scipy implementation, a 0-dimensional device array is returned instead here to avoid device->host transfers as much as possible.

I also suppressed all instances of cupy._util.PerformanceWarning occuring in the tests of this module that existed prior to this PR.

@emcastillo emcastillo self-assigned this Nov 24, 2020
@kmaehashi kmaehashi added cat:feature New features/APIs prio:medium labels Nov 25, 2020
@emcastillo
Copy link
Member

sorry to ask this, but could you please align the tests to those of #4307?

Thanks!

@grlee77
Copy link
Contributor Author

grlee77 commented Dec 1, 2020

sorry to ask this, but could you please align the tests to those of #4307?

Sure. I will do it for the recent fourier_ellipsoid PR as well.

@grlee77 grlee77 force-pushed the ndimage_measurements_center_of_mass branch from 55be4ca to 50debde Compare December 7, 2020 21:01
remove unused code path

use single quotes

remove misplaced quote

TST: make style of new tests consistent with changes in cupygh-4307
@grlee77 grlee77 force-pushed the ndimage_measurements_center_of_mass branch from 50debde to 5b11e68 Compare December 7, 2020 21:13
@grlee77
Copy link
Contributor Author

grlee77 commented Dec 7, 2020

rebased to fix merge conflict. should be ready for testing

Copy link
Member

@emcastillo emcastillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@emcastillo
Copy link
Member

TODO: Add docs

@emcastillo
Copy link
Member

Jenkins, test this please

@emcastillo emcastillo added this to the v9.0.0b1 milestone Dec 10, 2020
@emcastillo emcastillo added the st:test-and-merge (deprecated) Ready to merge after test pass. label Dec 10, 2020
@grlee77
Copy link
Contributor Author

grlee77 commented Dec 10, 2020

TODO: Add docs

I have already added this one to the API docs.

@chainer-ci
Copy link
Member

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

@mergify mergify bot merged commit fc35344 into cupy:master Dec 10, 2020
@grlee77 grlee77 deleted the ndimage_measurements_center_of_mass branch December 18, 2020 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:feature New features/APIs prio:medium st:test-and-merge (deprecated) Ready to merge after test pass.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants