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

Coordinate descent should work on float32 data (in addition to float64 data) #5464

Closed
arthurmensch opened this issue Oct 19, 2015 · 8 comments

Comments

@arthurmensch
Copy link
Contributor

For large scale application, constraining ElasticNet input to be float64 is a waste of space, as same quality results should be obtainable with float32 or event float16 input.

Use of cython in coordinate descent module make the type flexibility a bit tricky, but should be doable.

@arthurmensch arthurmensch changed the title Coordinate descent should be able to use float32 Coordinate descent should be able to use float32 or float16 Oct 19, 2015
@arthurmensch arthurmensch changed the title Coordinate descent should be able to use float32 or float16 Coordinate descent should work on float32 and float16 Oct 19, 2015
@GaelVaroquaux
Copy link
Member

GaelVaroquaux commented Oct 19, 2015 via email

@ogrisel
Copy link
Member

ogrisel commented Oct 20, 2015

Float16 is too low: BLAS does not support it. But float32 (in addition to float64) is a good idea.

BLAS comes in 2 flavors (for operations on real numbers as in non-complex): single precision (float32) and double precision (float64). The single precision operation are prefixed with s (e.g. saxpy and sgemm) while double precision operations are prefixed with d (e.g. daxpy and dgemm).

@ogrisel ogrisel changed the title Coordinate descent should work on float32 and float16 Coordinate descent should work on float32 data (in addition to float64 data) Oct 20, 2015
@MechCoder
Copy link
Member

I can work on this, if noone is already!

@arthurmensch
Copy link
Contributor Author

Sure, but we need to remove c files from the repository before, so as not
to blow repository size with generated c files from fused type.
Le 20 oct. 2015 23:11, "Manoj Kumar" notifications@github.com a écrit :

I can work on this, if noone is already!


Reply to this email directly or view it on GitHub
#5464 (comment)
.

@MechCoder
Copy link
Member

Oh the idea is to make the build script generate c from pyx files locally instead of them being the repository?

[Sorry, if there was a discussion but I missed it]

@arthurmensch
Copy link
Contributor Author

Yes, checkout PR #5492. Still WIP !

@amueller
Copy link
Member

@MechCoder but you can work on it independently. We just need #5492 be done before merging this.

I didn't realize you could do if typeof(x) is double with fused types, but apparently that's the way to do it.

@yenchenlin
Copy link
Contributor

Hello @MechCoder ,
I've already go through #6430, may I take a stab at this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants