Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(perf): Distribute data types inference #1692

Merged
merged 4 commits into from
Oct 18, 2022
Merged

(perf): Distribute data types inference #1692

merged 4 commits into from
Oct 18, 2022

Conversation

jaidisido
Copy link
Contributor

Feature or Bugfix

  • Performance

Current data types inference is done on the entire dataframe causing modin to pull the data into the driver and then loop over each column which is extremely inefficient.

Two options are available:

  1. Infer the data types from the first block or sample the data. It's efficient but risky because it would be based on a smaller sample size leading to incorrect inference (e.g. sparse data)
  2. Infer the data types from each modin block and combine them. It's thorough however we also run the risk of infering different data types for a column in two different blocks.

The current PR implements option 2

Detail

  • Distribute data types inference based on Modin blocks

Relates

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jaidisido jaidisido self-assigned this Oct 17, 2022
@jaidisido jaidisido added this to the 3.0.0 milestone Oct 17, 2022
@jaidisido jaidisido added enhancement New feature or request major release Will be addressed in the next major release labels Oct 17, 2022
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: b7eaa31
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido jaidisido linked an issue Oct 17, 2022 that may be closed by this pull request
(bug) modin pyarrow type inference #1681
Closed
jaidisido
jaidisido commented Oct 17, 2022
View reviewed changes
awswrangler/distributed/ray/modin/_data_types.py Outdated
for object_ref in block_object_refs
]
)
return {k: v for d in result for k, v in d.items()}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: The last block(s) are likely to overwrite data types

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make use of pyarrow.unify_schemas function. If the data types are incompatible, it will just fail. But catching the exception will at least give us a chance to log the error along with details.

The log can then be something like:

Data schemas across data blocks are incompatible. Sampling to one of the schemas

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: b7eaa31
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: b7eaa31
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 232ed71
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 232ed71
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 232ed71
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

LeonLuttenberger
LeonLuttenberger reviewed Oct 17, 2022
View reviewed changes
awswrangler/distributed/ray/modin/_data_types.py Outdated
# Dictionaries in list_col_types might not be equal (i.e. different col types in different blocks)
# In which case we return the most frequent value for each key
# More details here: https://github.com/aws/aws-sdk-pandas/pull/1692
keys = set().union(*(d.keys() for d in list_col_types))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to still use the pyarrow.unify_schemas function? It would be good just to check that the schemas are different, and to log a warning.

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 5b5795a
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 5b5795a
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 5b5795a
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 2ae5ca3
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 2ae5ca3
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@kukushking
Copy link
Contributor

Awesome! Real nice to see how easy it is to hook up alternative distributed functions

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 2ae5ca3
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 2ae5ca3
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido jaidisido merged commit 875e2ce into release-3.0.0 Oct 18, 2022
@jaidisido jaidisido deleted the perf/infer-data-types-to-parquet branch October 18, 2022 16:35
@kukushking kukushking moved this from In Review to Done in AWS SDK for pandas roadmap Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeonLuttenberger LeonLuttenberger LeonLuttenberger left review comments

@kukushking kukushking kukushking approved these changes

@cnfait cnfait Awaiting requested review from cnfait

@malachi-constant malachi-constant Awaiting requested review from malachi-constant

Assignees

@jaidisido jaidisido

Labels
enhancement New feature or request major release Will be addressed in the next major release performance
Projects
AWS SDK for pandas roadmap
Done
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

(bug) modin pyarrow type inference
4 participants
@jaidisido @malachi-constant @kukushking @LeonLuttenberger