gonum/floats

Remove AddConst

vladimir-ch opened this issue · 5 comments

vladimir-ch commented

In #27 @btracey mentioned that AddConst should be removed. It is just a wrapper around a simple loop but it is used at a couple of places in gonum (some of them added recently), so it looks like it is doing some service. What is the current opinion on this?

Kunde21 commented

It could be removed or added to internal/asm in vectorized form like this to add some speed.

kortschak commented

@Kunde21 would you put together a PR for that with benchmarks?

Kunde21 commented

I've been using some of these functions for vectorization practice. I've got these lying around in different scratch files:
CumSum, CumProd, Div, Mul, Sum, Scale, AddConst, Distance(L=1), Distance(L=+Inf)

Some can be pulled from numgo with minor adjustments to remove the ndarray broadcasting inherant to that library. I'll dig them up and pull them all into a PR with benchmarks (~3x speedup is the usual). Should I send a PR for both floats and asm with implementations and calls?

I'm going to work on Equal, EqualApprox, Same, HasNaN, Max, Min, and Span in my free time.

kortschak commented

I think the path here is to first send a post to gonum-dev indicating the intention and then given people are happy with it (I can see no real reason not to be) then send a PR to internal/asm and then here using the asm code - no assembler in floats directly.

btracey commented

It's probably not so terrible to have, especially since it's used in other places. I'm closing this. I think future additions should be subject to the "does it treat the data as a whole"