FIX-#3981, FIX-#3801, FIX-#4149: Stop broadcasting scalars to set items. #4160
Conversation
This file contains 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 #4160 +/- ##
==========================================
+ Coverage 86.80% 89.72% +2.91%
==========================================
Files 201 201
Lines 16795 16797 +2
==========================================
+ Hits 14579 15071 +492
+ Misses 2216 1726 -490
Continue to review full report at Codecov.
|
|
|
|
mvashishtha
force-pushed
the
3981-fix-assign-none-to-string-array
branch
from
89329e1
to
e2cbe97
Compare
dchigarev
reviewed
Feb 8, 2022
dchigarev
reviewed
Feb 8, 2022
added 3 commits
February 8, 2022 11:51
…lars to set items. Signed-off-by: mvashishtha <mahesh@ponder.io>
Signed-off-by: mvashishtha <mahesh@ponder.io>
Signed-off-by: mvashishtha <mahesh@ponder.io>
mvashishtha
force-pushed
the
3981-fix-assign-none-to-string-array
branch
from
e2cbe97
to
b7bf0bd
Compare
YarShev
reviewed
Feb 16, 2022
modin/core/execution/ray/implementations/pandas_on_ray/partitioning/partition_manager.py
Outdated
Show resolved
Hide resolved
…ile. Signed-off-by: mvashishtha <mahesh@ponder.io>
@YarShev Done! |
YarShev
approved these changes
Feb 22, 2022
|
As I see, this PR should resolve 3 issues. Please correct the title. |
@YarShev which 3 issues are you thinking of? I think it's only #3981 and #3801. The other issues I mentioned in the first comment are in pandas, or they are already fixed in Modin. |
mvashishtha
changed the title
vnlitvinov
pushed a commit
that referenced
this pull request
Mar 17, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Signed-off-by: mvashishtha mahesh@ponder.io
What do these changes do?
Normally, when assigning assigning a scalar to part of a dataframe, we broadcast the scalar to an array that is as big as the portion of the dataframe being modified. This causes a few problems:
Noneornp.NaNto astringseries or dataframe: Value error when trying to access a series having dtype: string and some elements set to None #3981None.MODIN_BENCHMARK_MODE=True:I ran this with Dask on my computer, which is:
With broadcasting, the assignment took an average of 75.5 seconds on two runs, while without brodacsting, it took an average of 13.5 seconds on two runs.
I don't see any benefit to broadcasting scalars and then slicing the resulting array up for each partition, so we should stop doing that.
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.pyblack --check modin/ asv_bench/benchmarks scripts/doc_checker.pygit commit -sdocs/development/architecture.rstis up-to-date