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

big_O is buggy #1368

Open
anurudhp opened this issue Aug 29, 2024 · 1 comment
Open

big_O is buggy #1368

anurudhp opened this issue Aug 29, 2024 · 1 comment
Labels
Milestone

Comments

@anurudhp
Copy link
Contributor

big_O only takes into account all variables in the input expression (setting their limit to inf), but not once multiplied later.

import sympy
from qualtran.resource_counting import big_O

n = sympy.Symbol("n", positive=True)

print(big_O(1) * n) # O(n)
print(big_O(1) * (1 / n)) # O(1/n)
print(big_O(1) * (n + 1)) # O(1)
print(big_O(1) * (1 / n + 1)) # O(1/n)

Also, sympy does not support multivariate orders. And as we usually have variables tending to both limits, e.g. n, m etc to inf, and eps, delta etc to 0, using big_O might lead to incorrect expressions.


One option would be to instead return C * E instead of big_O(E) in call graphs etc. And while viewing expressions, the user can take big_O of the whole thing. Could possibly use SympySymbolAllocator to generate these constants.

@mpharrigan
Copy link
Collaborator

You mean just include an arbitrary constant $C$ in the returned expression for a bloq?

@mpharrigan mpharrigan added this to the v1.0 milestone Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants