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

Optimized saturation operator #22

Merged
merged 12 commits into from May 2, 2016
Merged

Optimized saturation operator #22

merged 12 commits into from May 2, 2016

Conversation

perrygeo
Copy link
Contributor

Resolves #21

The previous saturation operation went something like:

  • swap axes to image order
  • convert colorspace using skimage
  • swap axes to rasterio order
  • apply saturation mutliplier
  • swap axes to image order
  • convert colorspace using skimage
  • swap axes to rasterio order

This PR provides a cython rio_color.colorspace module which provides c functions to do the math directly, then some python wrappers to work efficiently against ndarrays.

See #21 for details of testing and benchmarks.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 92e63fc on colorspaces into 2ea15fe on master.

@perrygeo
Copy link
Contributor Author

@sgillies could I get your 👀 on the Cython code in colorspace.pyx? Don't worry about checking the math, I'd just like to get some general feedback on the approach.

  • private cdef funcs that return C structs and do the actual colorspace math
  • python functions to wrap the private cdef funcs and return tuples
  • np.ndarray-typed cpdef functions using loops to apply colorspace conversions to arrays, one of which does the rgb->lch->rgb round trip with saturation adjustment.

Ping me if you see anything that we could do more safely, concisely or efficiently.

cdef int a0 = arr.shape[0]
cdef int a1 = arr.shape[1]
cdef int a2 = arr.shape[2]
cdef np.ndarray[FLOAT_t, ndim=3] out = np.zeros((a0, a1, a2))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find any advice or benchmarks on the best way to allocate an array of equal shape. This works but there may be more efficient ways.

@virginiayung virginiayung mentioned this pull request Apr 25, 2016
3 tasks
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 33578a0 on colorspaces into e1dfb37 on master.

@perrygeo perrygeo changed the title [WIP] Optimized saturation operator Optimized saturation operator Apr 27, 2016
@coveralls
Copy link

coveralls commented May 2, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 3937546 on colorspaces into e1dfb37 on master.

@coveralls
Copy link

coveralls commented May 2, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling cdfe44d on colorspaces into e1dfb37 on master.

@perrygeo
Copy link
Contributor Author

perrygeo commented May 2, 2016

@sgillies typing the loop variables didn't do much for speed but changing the order did.

I looked into memoryviews. The primary benefit seems to be writing generic functions that can use cython, numpy or C arrays. I tried memoryviews and it works but there's no speed difference and I don't see the benefit - dealing with numpy arrays explicitly seems like a cleaner approach.

@coveralls
Copy link

coveralls commented May 2, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 5e4a01d on colorspaces into e1dfb37 on master.

@perrygeo perrygeo merged commit 7cc5370 into master May 2, 2016
@perrygeo perrygeo deleted the colorspaces branch May 2, 2016 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants