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

Correctness issue #44

Open
wsmoses opened this issue May 1, 2024 · 5 comments
Open

Correctness issue #44

wsmoses opened this issue May 1, 2024 · 5 comments

Comments

@wsmoses
Copy link

wsmoses commented May 1, 2024

primal = func.val(body.val, alg.val, model.val, range.val)

@michel2323 this line seems to have a correctness issue. It's not necessarily the cause of a different issue @swilliamson7 and I were debugging, but seems sufficiently bad that it could be the culprit.

In essence you're only running the body of the loop (func.val) if the return is needed. However, if, for example, the return is nothing and the body of the fnuction updates something in place, your rule will tell Enzyme to not execute the original body of the function which is wrong.

cc @vchuravy

@wsmoses
Copy link
Author

wsmoses commented May 1, 2024

The fix should basically just be moving func.val before the if statement

@wsmoses
Copy link
Author

wsmoses commented May 1, 2024

Same thing for the while loop, presumably

michel2323 added a commit that referenced this issue May 2, 2024
michel2323 added a commit that referenced this issue May 2, 2024
@michel2323
Copy link
Member

Should be fixed #45 . Though when doing multilevel checkpointing I still have an issue with the primal. Gotta look into it.

@swilliamson7
Copy link

Hopefully okay now, thank you Michel! I need to check derivatives, but they aren't zero anymore which is good 😊

@michel2323
Copy link
Member

Fingers crossed. Ping me if not! @swilliamson7

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

No branches or pull requests

3 participants