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.

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

Tighter bounds for IntMap and IntSet merge #1110

Merged
merged 2 commits into from
Feb 23, 2025

Conversation

meooow25
Copy link
Contributor

Closes #1011

@meooow25 meooow25 requested review from treeowl and Lysxia February 21, 2025 14:34
@meooow25
Copy link
Contributor Author

It would be good to get some more eyes on this.

Copy link
Contributor

@Lysxia Lysxia left a comment

Choose a reason for hiding this comment

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

Your analysis looks good to me.

It may be worth explicitly defining $n$ as the min and $m$ as the max of the two sizes, otherwise at first glance it looks like there is something asymmetric going on.

@meooow25 meooow25 changed the title Document tighter bounds for IntMap and IntSet merge Tighter bounds for IntMap and IntSet merge Feb 22, 2025
@meooow25
Copy link
Contributor Author

$O(\min(n, m \log \frac{2^W}{m})), m \leq n$

It does say $m \leq n$, in the same manner as Set and Map.

Though maybe you mean one may mistakenly think $m$ and $n$ are the first and second maps?

@Lysxia
Copy link
Contributor

Lysxia commented Feb 22, 2025

Yes that's what I mean.

@meooow25
Copy link
Contributor Author

Hmm that goes for Set and Map too. And I'm not sure how to make it better.

$O(\min(\max(m,n), \min(m,n) \log \frac{2^W}{\min(m,n)}))$ looks terrible.

@Lysxia
Copy link
Contributor

Lysxia commented Feb 23, 2025

How about a top level comment under the "Combine" header, with a slight change of naming:

Most of these combining functions have complexity $O(\min(N, n \log(\frac{2^W}{n})))$, where $n$ is the minimum of the sizes of the arguments and $N$ is their maximum.

@meooow25 meooow25 force-pushed the intmap-merge-complexity branch from 7a844c6 to dfd3f0c Compare February 23, 2025 16:05
@meooow25
Copy link
Contributor Author

meooow25 commented Feb 23, 2025

The "Combine" section might not be enough, since a few functions like isSubmapOf are elsewhere. But I like the idea, we can put this comment at the top of the module instead.

I'll make a separate PR for it (with Set and Map too).

Edit: Oh and I would like to keep the $m$ and $n$ names (rather than $n$ and $N$) because the terms are from the "Parallel Ordered Sets Using Join" paper and it's nice to be consistent. $N$ is also used to denote a maximum value in #957, which could be confusing.

@meooow25 meooow25 merged commit 369fb4b into haskell:master Feb 23, 2025
13 checks passed
@meooow25 meooow25 deleted the intmap-merge-complexity branch February 23, 2025 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complexity of IntMap-IntMap operations
2 participants