Skip to content

PERF: MultiIndex.union #48752

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

Merged
merged 5 commits into from
Oct 14, 2022
Merged

PERF: MultiIndex.union #48752

merged 5 commits into from
Oct 14, 2022

Conversation

lukemanley
Copy link
Member

cc @phofl - following your improvements to MultiIndex.difference, it looks like we can replace fast_unique_multiple.

     [57630237]       [7d8e3cf8]
     <main>           <multiindex-union>
-      80.0±0.9ms       61.7±0.6ms     0.77  multiindex_object.SetOperations.time_operation('non_monotonic', 'string', 'union')
-        81.7±1ms         61.1±1ms     0.75  multiindex_object.SetOperations.time_operation('monotonic', 'string', 'union')
-      55.6±0.4ms       38.3±0.3ms     0.69  multiindex_object.SetOperations.time_operation('non_monotonic', 'ea_int', 'union')
-        53.8±1ms       36.6±0.8ms     0.68  multiindex_object.SetOperations.time_operation('monotonic', 'ea_int', 'union')
-      34.1±0.4ms       14.2±0.6ms     0.42  multiindex_object.SetOperations.time_operation('non_monotonic', 'int', 'union')
-        33.9±2ms       13.8±0.6ms     0.41  multiindex_object.SetOperations.time_operation('monotonic', 'int', 'union')
-        75.8±4ms       14.9±0.4ms     0.20  multiindex_object.SetOperations.time_operation('non_monotonic', 'datetime', 'union')
-        75.4±3ms       14.3±0.6ms     0.19  multiindex_object.SetOperations.time_operation('monotonic', 'datetime', 'union')
- ```

@lukemanley lukemanley added Performance Memory or execution speed performance MultiIndex labels Sep 23, 2022
right_missing = lib.fast_unique_multiple(self._values, rvals)
if right_missing:
result = self.append(other.take(right_missing))
right_missing = other.difference(self)
Copy link
Member

Choose a reason for hiding this comment

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

How does this behave with duplicates and especially with Indexes that contain tuples?

Apart from that, I would prefer not doing that now. I have a different pr that I would prefer merging first.

Copy link
Member Author

Choose a reason for hiding this comment

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

Duplicates shouldn’t reach this given the if else block this is within (other is always unique). Not sure about tuples but I can take a look. Or I can just close it if you have something in the works.

Copy link
Member

Choose a reason for hiding this comment

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

Not really in the works yet, but I would prefer optimising the duplicates in other case before we optimise here. This is more needed (or you could look into that if you like, I suspect that we have to push that into the cython layer to get an acceptable performance improvement)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I'll take a look at the duplicates. I'll leave this open a bit longer.

Copy link
Member Author

Choose a reason for hiding this comment

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

@phofl, are you open to this one now that the duplicates case has been optimized a bit?

Copy link
Member

Choose a reason for hiding this comment

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

Can you rebase now?

@phofl phofl added this to the 2.0 milestone Oct 14, 2022
@phofl phofl merged commit e3d0126 into pandas-dev:main Oct 14, 2022
@phofl
Copy link
Member

phofl commented Oct 14, 2022

thx @lukemanley

@lukemanley lukemanley deleted the multiindex-union branch October 26, 2022 10:18
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MultiIndex Performance Memory or execution speed performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants