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

Added 100x faster vectorized version #39

Merged
merged 1 commit into from Jul 5, 2016
Merged

Added 100x faster vectorized version #39

merged 1 commit into from Jul 5, 2016

Conversation

parejkoj
Copy link
Contributor

@parejkoj parejkoj commented Jul 4, 2016

Would be ~500x faster if we didn't have to do the footprint loop.

The if branch between vector and non-vectorized versions (and associated split in code logic) is unpleasant, but we'll have to live with it until we finalize the discussion here: https://community.lsst.org/t/memory-contiguity-and-python-execution-speed/884/12

return Struct(sourceCat=result)

def _getSchemaKeys(self, schema):
"""Extract and save the necessary keys from schema with asKey."""
self.parentKey = schema["parent"].asKey()
self.nChildKey = schema["deblend_nChild"].asKey()
self.centroidXKey = schema["slot_Centroid_x"].asKey()
self.centroidYKey = schema["slot_Centroid_y"].asKey()
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the field was "slot_Centroid" and it returned a tuple of the (y, x) centroid location. Does this also work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That works for individual sources, but I need the x/y centroids for the vectorized np.isfinite() test: there isn't an equivalent SourceCatalog thing to get a vector of tuples.

Copy link
Member

Choose a reason for hiding this comment

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

Small correction: when used as a single key slot_Centroid will return afw.geom.Point2D, which is indexable as (x,y), not (y,x). The usage here is completely valid too, of course.

@parejkoj parejkoj merged commit 5bd07ac into master Jul 5, 2016
@ktlim ktlim deleted the tickets/DM-5933 branch August 25, 2018 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants