CI Testing: Set Memory Layout On Purpose 
State: closed

- [ ] closes #26502
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Groupby
name: Performance
description: Memory or execution speed performance

Comments:
Hello @TannhauserGate42! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:











There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2019-05-27 14:54:35 UTC
Is the idea here to let the user specify a preferred default layout?  That could go in e.g. pd.options.

Needs tests.
> Is the idea here to let the user specify a preferred default layout? That could go in e.g. pd.options.
> 
> Needs tests.

I will provide some. First, I need to succeed to check consistency and get through the unittests ... it might have wide reaching dependencies. Need to get the unittest suite running properly on my machines. ... not a pro, sorry. 

I need to understand some tests first. changing memory layout might have consequences especially in conjunction with pytables tests, should there be implicit assumptions on the memory layout given so far. I hope, it does not. ... but hope is not a method. 
Just a short FYI, this may not be something we can actually guarantee going
forward. There's a possibility that pandas' internal representation will
change from a mixture of 1D and 2D arrays
to just 1D arrays (though the 1D arrays may be views on a 2D array). All
that is quite uncertain though.

On Tue, May 28, 2019 at 2:22 AM Thomas Siller <notifications@github.com>
wrote:

> Is the idea here to let the user specify a preferred default layout? That
> could go in e.g. pd.options.
>
> Needs tests.
>
> I will provide some. First, I need to succeed to check consistency and get
> through the unittests ... it might have wide reaching dependencies. Need to
> get the unittest suite running properly on my machines. ... not a pro,
> sorry.
>
> I need to understand some tests first. changing memory layout might have
> consequences especially in conjunction with pytables tests, should there be
> implicit assumptions on the memory layout given so far. I hope, it does
> not. ... but hope is not a method.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/26538?email_source=notifications&email_token=AAKAOIRJT26BUKE4AKKR4NTPXTMRXA5CNFSM4HP4PDZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWLGQRY#issuecomment-496396359>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIRCS3SX27AYVJV2HJTPXTMRXANCNFSM4HP4PDZQ>
> .
>

@TannhauserGate42 can you indicate how this is different to the previous PR (https://github.com/pandas-dev/pandas/pull/26529)? If there is no difference: practical point of feedback: it is not needed to open new PRs when updating based on feedback.

But, I agree with @TomAugspurger that we might not necessarily want to include something like this. So I would recommend to first focus on actually discussing this before putting too much effort in a PR (although for the discussion, having benchmarks might be needed, which might need an initial implementation).
> @TannhauserGate42 can you indicate how this is different to the previous PR (#26529)? If there is no difference: practical point of feedback: it is not needed to open new PRs when updating based on feedback.
> 
> But, I agree with @TomAugspurger that we might not necessarily want to include something like this. So I would recommend to first focus on actually discussing this before putting too much effort in a PR (although for the discussion, having benchmarks might be needed, which might need an initial implementation).

Thanks. 

I have the testsuite now running locally and will try another concept for our own purpose. Thanks for letting me use your CI for testing. 

I aggree, that my implementation is not the one to go forward and is not meant as such. At the same time it shows me, that pandas implicitly relies on some numpy memory layout and lots of stuff would brake, if these implicit assumptions (like np-defaulting to C-order) would not hold anymore.

It would be good, if pandas would allow to set the memory layout or at least would be consistently always use the same (which I showed is not true).

Unfortunatly, we have severer problems with performance now and with a quick fix I went down from 1:25 hours to 17 minutes. All because aggregation on the one and same dataframe takes about 5 minutes with F-order and about 20 seconds with C-order memory layout. 

I leave you with the question: Why does pandas indexing depend on the memory layout of numpy ndarrays?