FEAT-#1205: melt implementation #1689
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1689 +/- ##
==========================================
- Coverage 82.32% 81.75% -0.58%
==========================================
Files 77 77
Lines 8189 8867 +678
==========================================
+ Hits 6742 7249 +507
- Misses 1447 1618 +171
Continue to review full report at Codecov.
|
|
|
|
Collaborator
devin-petersohn
left a comment
devin-petersohn
left a comment
There was a problem hiding this comment.
Do you have performance on this? Looking through the implementation, it might be expensive.
Collaborator
Author
|
@devin-petersohn Thanks for your comments, definitely first implementation were extremely slow because of inserting column with As we see from the results, native modin frame function Here is the results without restoring pandas-order: How it was measuredimport numpy as np
import string
import csv
import os
from timeit import default_timer as timer
IS_PANDAS = True
RAND_LOW = -100
RAND_HIGH = 100
N = 500000
M = 45
TEST_FILENAME = os.path.abspath(f"int_dataset-{N},{M},{RAND_LOW},{RAND_HIGH}.csv")
def generate_data_file(filename, row_n, col_n):
letters = list(string.ascii_letters)
columns = [
"".join(np.random.choice(letters) for _ in np.arange(10))
for _ in np.arange(col_n)
]
with open(filename, "w") as file:
writer = csv.writer(file, delimiter=",")
writer.writerow(columns)
for _ in np.arange(row_n):
writer.writerow(np.random.randint(RAND_LOW, RAND_HIGH, col_n))
if __name__ == "__main__":
import pandas
import modin.pandas as pd
if not os.path.exists(TEST_FILENAME):
generate_data_file(TEST_FILENAME, N, M)
if IS_PANDAS:
pd_df = pandas.read_csv(TEST_FILENAME)
t1_pd = timer()
pd_df.melt()
t2_pd = timer()
print("Pandas:", t2_pd - t1_pd)
md_df = pd.read_csv(TEST_FILENAME)
t1_md = timer()
md_df.melt()
t2_md = timer()
print("Modin:", t2_md - t1_md)What is restoring pandas-order?For applying For example we have DataFrame splitter into row partitions like that: and then we will apply Restoring order from right result in left result is what I mean under restoring pandas-order Restoring order is the most expensive operation because we must have knowledge about the whole column axis, which is the longest one in case of So, @devin-petersohn is it acceptable for modin to output shuffled data in such functions or modin has a pattern for fast reordering which I'm don't know? |
Collaborator
|
@dchigarev maybe you can found some useful things in #725 |
Collaborator
devin-petersohn
left a comment
devin-petersohn
left a comment
There was a problem hiding this comment.
_apply_full_axis is asynchronous. Timing before and after isn't enough, you also have to print the result to block. It is 4+x slower than a MapFunction.
I left some questions for you.
Collaborator
Author
|
@devin-petersohn I've reimplemented I've also fixed a strange bug in |
dchigarev
changed the title
Collaborator
devin-petersohn
left a comment
devin-petersohn
left a comment
There was a problem hiding this comment.
@dchigarev Yes, if there is no work being done, the column partition is still shuffled together to the same worker, which is some significant work.
Thanks for running the performance numbers on this. Is there a reason this cannot be done by applying the function to row partitions instead of column partitions?
Collaborator
Author
@devin-petersohn It could be implemented via applying to row partitions, but there is a two details:
|
Collaborator
Ah, I forgot about that, sorry. The reordering rows can happen via sort, which isn't implemented scalably yet, but should be soon. It is okay to rely on functionality that isn't implemented yet. We could also perform the sort first, which would probably perform better. What do you think about this? |
Collaborator
Author
|
@devin-petersohn what do you mean under "perform the sort first"? The problem is that after applying And another question, I don't know how data scientists use |
Collaborator
|
@dchigarev It will scale better if operations are performed across the columns |
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Collaborator
Author
|
@devin-petersohn I've reimplemented Also added a warning of incorrect order of rows and created an issue, to we could fix this later (#1793) |
Collaborator
|
Thanks @dchigarev! Were those times including a print statement to ensure computation was finished? |
Collaborator
Author
|
@devin-petersohn It's shamable, but it seems that I forgot to add |
Collaborator
|
@dchigarev 500,000 rows is not very many, so that speedup is quite good, especially on 128 columns. |
Collaborator
|
@dchigarev The implementation looks good, I have retriggered GitHub Actions because it showed a -40% coverage drop. |
Collaborator
Author
|
@devin-petersohn now codecov check is okay, I guess |
devin-petersohn
approved these changes
Jul 24, 2020
Collaborator
devin-petersohn
left a comment
devin-petersohn
left a comment
There was a problem hiding this comment.
Thanks @dchigarev, looks great!
devin-petersohn
changed the title
aregm
pushed a commit
to aregm/modin
that referenced
this pull request
Sep 16, 2020
Signed-off-by: Dmitry Chigarev <dmitry.chigarev@intel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do these changes do?
Implementation of
pandas.DataFrame.melt+apply_full_axis_select_indicesoptimization.flake8 modinblack --check modingit commit -s