Skip to content

POC Use fused types more, tempita less #22432

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

Closed
wants to merge 5 commits into from

Conversation

jbrockmendel
Copy link
Member

Low priority after other outstanding cython PRs; will run asvs before long.

Discussed in #22411.

@WillAyd
Copy link
Member

WillAyd commented Aug 20, 2018

Haven't reviewed in detail yet but high level would you still expect this code to live in the .pxi.in file? I was assuming we'd be able to do away with the template file altogether and simply migrate the code into a single .pyx file (here algos_common.pyx)

@jbrockmendel
Copy link
Member Author

jbrockmendel commented Aug 20, 2018

Haven't reviewed in detail yet but high level would you still expect this code to live in the .pxi.in file?

No, for the real version of this the affected functions would be moved out of the pxi.in files into the regular pyx files. While in progress it's easier to cut and paste within the same file.

@jreback
Copy link
Contributor

jreback commented Aug 20, 2018

so my 2 question are:

  1. perf - should be very close
  2. are there any blockers to getting rid of tempita completely (obvioulsy thru multiple PRs). maybe try some harder ones, e.g. hashtable and see.

@jbrockmendel
Copy link
Member Author

perf - should be very close

I'll run some asvs before long. I'd like to clear up the other cython PRs (especially the fairly easy #22329) before diving into this too deep.

are there any blockers to getting rid of tempita completely (obvioulsy thru multiple PRs). maybe try some harder ones, e.g. hashtable and see.

I ran into trouble in converting code in groupby_helper functions, so it definitely isn't trivial.

@gfyoung gfyoung added Refactor Internal refactoring of code Internals Related to non-user accessible pandas implementation labels Aug 21, 2018
@jbrockmendel
Copy link
Member Author

asvs for just the reshape functions:

These look like noise to me, erring on the side of slightly slower.

time taskset 8 asv continuous -f 1.1 -E virtualenv master HEAD -b reshape
[...]

    before     after       ratio
  [513c02cb] [5a4e7fc9]
+    1.41ms     2.15ms      1.52  reshape.SparseIndex.time_unstack
+    2.01ms     2.95ms      1.47  reshape.SimpleReshape.time_unstack
+    5.02ms     6.26ms      1.24  reshape.SimpleReshape.time_stack
+  129.36ms   152.83ms      1.18  reshape.Pivot.time_reshape_pivot_time_series
-   15.53ms    11.07ms      0.71  reshape.GetDummies.time_get_dummies_1d

    before     after       ratio
  [513c02cb] [5a4e7fc9]
-    5.24ms     4.40ms      0.84  reshape.Melt.time_melt_dataframe
-  396.79ms   329.76ms      0.83  reshape.WideToLong.time_wide_to_long_big
-  921.31ms   665.94ms      0.72  reshape.GetDummies.time_get_dummies_1d_sparse
-    5.49ms     3.81ms      0.69  reshape.SimpleReshape.time_stack

    before     after       ratio
  [513c02cb] [5a4e7fc9]
+   19.63ms    30.76ms      1.57  reshape.PivotTable.time_pivot_table
+  122.98ms   146.45ms      1.19  reshape.Pivot.time_reshape_pivot_time_series
-   15.63ms    14.05ms      0.90  reshape.GetDummies.time_get_dummies_1d
-  852.38ms   663.50ms      0.78  reshape.GetDummies.time_get_dummies_1d_sparse

    before     after       ratio
  [513c02cb] [5a4e7fc9]
+  327.94ms   436.79ms      1.33  reshape.WideToLong.time_wide_to_long_big
-  107.96ms    95.90ms      0.89  reshape.Unstack.time_full_product
-  908.24ms   760.68ms      0.84  reshape.GetDummies.time_get_dummies_1d_sparse
-    2.99ms     2.15ms      0.72  reshape.SimpleReshape.time_unstack
-   15.67ms    10.62ms      0.68  reshape.GetDummies.time_get_dummies_1d
-   29.09ms    19.43ms      0.67  reshape.PivotTable.time_pivot_table

    before     after       ratio
  [513c02cb] [5a4e7fc9]
+   19.04ms    31.11ms      1.63  reshape.PivotTable.time_pivot_table
+    1.49ms     2.13ms      1.43  reshape.SparseIndex.time_unstack
+  639.42ms   825.80ms      1.29  reshape.GetDummies.time_get_dummies_1d_sparse
+    2.28ms     2.84ms      1.25  reshape.SimpleReshape.time_unstack
+  219.25ms   243.20ms      1.11  reshape.Unstack.time_without_last_row
-    4.42ms     3.96ms      0.90  reshape.Melt.time_melt_dataframe
-  314.72ms   267.92ms      0.85  reshape.WideToLong.time_wide_to_long_big
-   14.47ms    10.95ms      0.76  reshape.GetDummies.time_get_dummies_1d

    before     after       ratio
  [513c02cb] [5a4e7fc9]
+   13.34ms    15.29ms      1.15  reshape.GetDummies.time_get_dummies_1d
+  317.12ms   355.70ms      1.12  reshape.WideToLong.time_wide_to_long_big
-    2.08ms     1.84ms      0.88  reshape.SparseIndex.time_unstack
-  910.80ms   741.24ms      0.81  reshape.GetDummies.time_get_dummies_1d_sparse

    before     after       ratio
  [513c02cb] [5a4e7fc9]
+   20.61ms    29.52ms      1.43  reshape.PivotTable.time_pivot_table
+  508.07ms   700.57ms      1.38  reshape.GetDummies.time_get_dummies_1d_sparse
+    3.56ms     4.78ms      1.34  reshape.Melt.time_melt_dataframe
+   86.99ms   109.69ms      1.26  reshape.Unstack.time_full_product
+    1.80ms     2.19ms      1.22  reshape.SparseIndex.time_unstack
-    6.44ms     5.80ms      0.90  reshape.SimpleReshape.time_stack
-  153.73ms   132.12ms      0.86  reshape.Pivot.time_reshape_pivot_time_series
-   15.70ms     9.44ms      0.60  reshape.GetDummies.time_get_dummies_1d

    before     after       ratio
  [513c02cb] [5a4e7fc9]
+  581.37ms   879.53ms      1.51  reshape.GetDummies.time_get_dummies_1d_sparse
+   91.89ms   109.90ms      1.20  reshape.Unstack.time_full_product
+  214.70ms   242.03ms      1.13  reshape.Unstack.time_without_last_row
-   31.03ms    27.06ms      0.87  reshape.PivotTable.time_pivot_table
-    5.18ms     4.20ms      0.81  reshape.Melt.time_melt_dataframe
-    5.88ms     4.02ms      0.68  reshape.SimpleReshape.time_stack

[100.00%] ··· Running reshape.WideToLong.time_wide_to_long_big                                                                                                 349.47ms    before     after       ratio
  [513c02cb] [5a4e7fc9]
+    3.36ms     4.20ms      1.25  reshape.Melt.time_melt_dataframe
+   87.05ms   104.05ms      1.20  reshape.Unstack.time_full_product
+    2.69ms     3.09ms      1.15  reshape.SimpleReshape.time_unstack
+  349.47ms   392.85ms      1.12  reshape.WideToLong.time_wide_to_long_big
-  135.33ms   122.51ms      0.91  reshape.Pivot.time_reshape_pivot_time_series
-   23.49ms    21.22ms      0.90  reshape.PivotTable.time_pivot_table
-    6.10ms     4.04ms      0.66  reshape.SimpleReshape.time_stack

   before     after       ratio
  [513c02cb] [5a4e7fc9]
+    9.50ms    15.85ms      1.67  reshape.GetDummies.time_get_dummies_1d
+    3.74ms     5.87ms      1.57  reshape.SimpleReshape.time_stack
-  793.39ms   700.72ms      0.88  reshape.GetDummies.time_get_dummies_1d_sparse
-  438.11ms   347.58ms      0.79  reshape.WideToLong.time_wide_to_long_big
-   31.55ms    23.60ms      0.75  reshape.PivotTable.time_pivot_table

@jbrockmendel
Copy link
Member Author

Closing this proof of concept, first real PR in this process is #22452.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internals Related to non-user accessible pandas implementation Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants