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

Memoization of closures #1099

Open
lars-reimann opened this issue Apr 24, 2024 · 0 comments
Open

Memoization of closures #1099

lars-reimann opened this issue Apr 24, 2024 · 0 comments
Labels
bug 🪲 Something isn't working

Comments

@lars-reimann
Copy link
Member

lars-reimann commented Apr 24, 2024

Describe the bug

Our code generation for calls with a closure as argument is wrong. We don't check whether the enclosed values changed, but just return the memoized result regardless.

To Reproduce

package bug

pipeline bug {
    val table = Table({
        "a": [1, 2]
    });
    val threshold = table.numberOfRows - 2; // Change this, observe result

    val result = table.filterRows((row) ->
        row.getValue("a") as Int > threshold
    );
}

Expected behavior

If an enclosed value changes, we should recalculate instead of looking up the result in the memoization table.

A quick fix would be that we don't memoize calls that have a lambda as an argument that references values outside of it.

Long term, we should just add those values as hidden arguments to the call, however. If the partial evaluator can already evaluate the reference, there's no need to add it as a hidden argument. We still need to add them to keep the order the same. Together with the generated Python code of the lambda, we must get something unique.

Screenshots (optional)

No response

Additional Context (optional)

No response

@lars-reimann lars-reimann added the bug 🪲 Something isn't working label Apr 24, 2024
@lars-reimann lars-reimann self-assigned this Apr 25, 2024
@lars-reimann lars-reimann modified the milestones: v0.14.0, v0.15.0 Apr 25, 2024
@lars-reimann lars-reimann removed their assignment May 5, 2024
@lars-reimann lars-reimann removed this from the v0.15.0 milestone May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working
Projects
Status: Todo
Development

No branches or pull requests

1 participant