ENH: Optimize take_*; improve non-NA fill_value support

[stephenwlin]

Optimizes/improves common.take_1d, common.take_2d, common.take_2d_multi, and common.take_fast in the following ways:

• For common cases, no intermediate buffer is required anymore to convert between two different dtypes (i.e. when upcasting is necessary): the data is taken and upcasted in one step. These cases are optimized in Cython and more can be added if desired.
• Better support for fill_value parameters other than NaN: previously, all upcasting was done assuming NaN as the fill_value: now, the correct upcast will be chosen based on the type of the fill_value. (Ex. integers stay as integers with integer fill_value if the value can fit in the existing size without overflow, integers and floats upcast to complex with complex fill_value, all numerics upcast to objects with boolean or string fill_value, etc.)
• Added option of fill_value of None, which means that the corresponding entries in the output are left unchanged (only really makes sense when out is also provided)

The last option is used to optimize internals._merge_blocks (i.e. the function internally called to do block consolidation): instead of vstack-ing the arrays together in an intermediate buffer and then rearranging into the output with a separate operation, the output buffer is allocated uninitialized and directly filled from the inputs using one call to common.take_fast per input block, even when they are not contiguous. (However, the special case of contiguous and already properly ordered blocks is still handed with vstack, since no other operation is required after that)

EDIT: took out fill_value of None option and reverted internals._merge_blocks: it wasn't actually helping. Upcasting within take functions is definitely faster though.

The existing vb_suite tests were not really testing upcasting, so I added one for it. Here are the results vs. master:

Results:
                                            t_head t_baseline      ratio
name
frame_reindex_upcast                       26.7874    34.3973     0.7788

and against jrebacks/dtypes:

Results:
                                            t_head t_baseline      ratio
name
frame_reindex_upcast                       23.8220    28.0891     0.8481

[jreback]

I updated dtypes a couple of times today
looks like u pulled in at least the first one, but not the last
I added int16/int8 some float16 support in cython. (generate_code.py)

good news is it looks like changes are for the most part independent so u can prob merge easily
(I changed astype and conver in internals/BlockManager)

[jreback]

jreback@e5b41db

was after you updated - amazing we actually worked on a small overlap at the same time!

[stephenwlin]

@jreback, rebased and resolved conflicts

EDIT: actually, need to add int8 and int16 support to common...doing that now

[jreback]

I think that the new way that u did the take dictionaries in order to specify the cython functions - need to add for int8/int16

I think the merge took your versions (which is correct), but need to add in those other functions
tests prob all pass - but the takes will be using the generic take instead of the cython takes for int8/int16
maybe add a test for this?

[jreback]

hah! again typed the same thing at the same time!

[jreback]

?

how are u running test_perf to get the vbench ratios - I can never get it to work?

[stephenwlin]

ok, merged with int8/int16 support...

i just run ./test_perf.sh -b {BASE_SHA} ... not sure why it doesn't work for you? also, I installed vbench from https://github.com/pydata/vbench

[jreback]

thanks I give it a try (I was putting too many options I think)

[jreback]

FYI - not sure if u r setup for Travis -

it picks up on a lot of dtype stuff and also shows how py3 react
(I actually installed py3.3 separately to test - it had some weird stuff)

[stephenwlin]

no, i'm not...how do i set that up?

[jreback]

http://about.travis-ci.org/docs/user/getting-started
step 2
it will auto build - I think u have to do something the first time though
then I click thru to the website to check progress and see how I fare on 3.3
and it's 32 bit
so u get really weird failures in some tests
eg if compre vs np.int_ with a specific dtype will fail (because its np.int32 on this platform)

[stephenwlin]

thanks for the travis info, will take a look tomorrow (hopefully I didn't break too much in 3.3)

[ghost]

has someone been playing with commit --author? I had nothing to do with df87f8f.

[stephenwlin]

i squashed all the commits in #2708 and somehow it showed up as yours, not sure why. if I do it again i'll try to update it to jreback. (EDIT: ok, I went ahead and amended)

[jreback]

I think you should squash my PR as well...ultimately yours will be merge on top of master (after mine)

@y-p or is that right, since he is useing my PR as a starting point (instead of master), I assume that updates/sync are all ok, but is there a difference in how you push/present?

[stephenwlin]

unless i'm mistaken about how things work, i think it's easier to keep it separate for now in case you update again: what i do to resolve is to reset back to master, merge your dtypes branch, squash everything into one commit, then cherry-pick the last version of my commit on top of that--it makes it easier to tell where the conflicts are that way.

after your PR is merged i will rebase and everything will be a single commit off the new master; should work fine, I think?

[ghost]

yep, exactly right. stephen keeps rebasing his commit on top of a squashed version of your rolling PR, and
when the time comes it'll get cherry picked after your PR is merged.

[jreback]

ahh...makes sense..thanks!

[jreback]

see you got travis running! great!

[jreback]

do you think we should add take support for uints? (and prob pad/fill and cases where I add ints now)?
its not hard....only thing is compile time gets longer....is this a big deal?

[jreback]

your generic takes handle this of course....question is should we include the cython specializations?

[stephenwlin]

i don't have an opinion, myself--i don't know enough about use cases. i'm fine updating the branch with whatever anyone else decides makes sense, though.

[jreback]

ok....minor push...removed support for float16...apparently cython really doesn't support these (and I was doing a conversion)...showed up in dff_int8 -> float16 (now goes to float32)...

also...you might want to add a test for _maybe_promote...something like:

dtypes = ['float16'.....,'int8'......'object']
for dtype in dtypes
     array_type in dtypes:
              # test whethere you are getting the correct promotion......

[jreback]

I rebased....hopefully didn't screw u up.....(and done unless doesn;'t build properly)

[stephenwlin]

test_2d_upcast_fill_nonna tests _maybe_promote indirectly...did you see that? if you think the coverage should be improved further let me know.

[jreback]

yes, of course.....that looks great.....its tricky to see diff's between the commits because of the dtypes changes.....but found it!

[stephenwlin]

Ahh, turns out _merge_blocks wasn't actually faster this way, so I reverted that part back...oh well. Upcasting is faster, though, for Cython-implemented cases, although current vbench tests don't really exercise it (will add a new test for it.)

[stephenwlin]

rebased off master after #2708 merged

[wesm]

FYI Cython optimizes range(n) so this won't have any effect

[stephenwlin]

yeah, i know...but there was a mix of the two between the templates so I decided to make it consistent...

[jreback]

@stephenwlin if you have a chance....can you take a quick look at #3089