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

Fix asymptotics of BasePattern #81

Merged

Conversation

uncomputable
Copy link
Collaborator

Depends on #80 (because of the inference context parameters)

Fixes #39 Fixes #40

@apoelstra
Copy link
Contributor

Can be rebased now. CI failure is "real" (cargo fmt). Nice!

@uncomputable uncomputable force-pushed the 2024-08-fix-asymptotics branch 2 times, most recently from b67a5d3 to 25f8109 Compare August 26, 2024 20:58
@uncomputable
Copy link
Collaborator Author

Rebased

@@ -206,40 +206,31 @@ impl BasePattern {
}
}

/// Check if the `identifier` is contained inside the pattern.
pub fn contains(&self, identifier: &Identifier) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

In 25f8109:

Might as well keep this method, though it can just be a oneliner self.get(identifier).is_some().

@apoelstra
Copy link
Contributor

In 25f8109:

The commit messages says the existing method was O(2^n) but you mean to say O(n^2) ... and really, O(n * depth).

Improve the asymptotics of BasePattern::get and of
BasePattern::contains_all to O(n).
@uncomputable uncomputable force-pushed the 2024-08-fix-asymptotics branch from 25f8109 to 8f3c948 Compare August 27, 2024 07:49
@uncomputable
Copy link
Collaborator Author

Addressed the comments

Copy link
Contributor

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 8f3c948 successfully ran local tests

@apoelstra apoelstra merged commit ec2b0a3 into BlockstreamResearch:master Aug 27, 2024
4 checks passed
@uncomputable uncomputable deleted the 2024-08-fix-asymptotics branch August 27, 2024 20:02
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.

O(n^2) contains_all method O(n^2) get method
2 participants