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

Fused type required for floating point Cython operations #3111

Closed
stefanv opened this issue May 28, 2018 · 6 comments
Closed

Fused type required for floating point Cython operations #3111

stefanv opened this issue May 28, 2018 · 6 comments
Milestone

Comments

@stefanv
Copy link
Member

stefanv commented May 28, 2018

Since we're now (soon ;) letting through float32 arrays in img_as_float, we need to add a fused dtype to all Cython code that consumes doubles to also accept single precision floats.

@grlee77
Copy link
Contributor

grlee77 commented May 29, 2018

Agreed, although some cases may need to use double precision internally for accuracy. An example is the change from float32 to float64 when using the fast integral-image representation for non-local means. A good image demonstrating the problem with float32 is here: #2878 (comment)

@jni
Copy link
Member

jni commented May 29, 2018

@grlee77 for those functions we should be using img_as_float64 instead of img_as_float.

@soupault soupault modified the milestones: 0.15, 0.16 Apr 20, 2019
@rfezzani
Copy link
Member

A big 👍 for this!
The np_floats fused type is already defined in skimage._shared.fused_numerics. Generalizing its use in Cython operations may in fact save memory use and computation time.
It also prevent some hidden data casting from float32 to double, like for example in _slic_cython where the input image is silently converted to double...

@rfezzani
Copy link
Member

Oups, I just realized that this issue is more than one year old!!! 😆

@stefanv
Copy link
Member Author

stefanv commented Oct 29, 2019

I think we still need someone to do a proper review of all changes required; but since this issue is so vague I'm going to close it.

@stefanv stefanv closed this as completed Oct 29, 2019
@rfezzani
Copy link
Member

I started this review yesterday when I noticed the silent cast to double in _slic_cython. That's how I found this discussion :-)
I will start by reporting the _slic_cython issue and refer to this one. I will try to investigate more on this.

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

No branches or pull requests

5 participants