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 regression with cache #95

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Conversation

jcjaskula-aws
Copy link
Collaborator

This change modified the cache objects. We were not caching the ast nodes anymore but the ouput of to_cached_oqpy_expression. This led to regression in benchmarks.

Solution: we store the ast in Program.expr_cache. I changed to_cached_oqpy_expression return type to be AstConvertible to match to_oqpy_expression. Consequently, we need to call the main to_ast function instead of HasToAst.to_ast.

Updated a unit test.

@jcjaskula-aws jcjaskula-aws force-pushed the jcjaskula-aws/fix_cache_regression branch from 93d10cd to 34326d1 Compare September 26, 2024 20:07
@PhilReinhold
Copy link
Collaborator

Looks good to me, but this could have been caught by mypy I think. The problem is that currently our lock file has openpulse=0.5.0 which does not export typing. We should try to fix this at some point by updating openpulse in our lock file and fixing the resulting mypy errors. We should probably also add a py.typed file so that downstream packages can use oqpy's type hints.

@PhilReinhold PhilReinhold merged commit 07d027b into main Sep 26, 2024
10 checks passed
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.

2 participants