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

[llvm] Fix initial observation value on env.reset(). #126

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

ChrisCummins
Copy link
Contributor

@ChrisCummins ChrisCummins commented Mar 11, 2021

Fixes a regression introduced in 6cccf05 that meant that env.reset() would not return an observation value for LlvmEnv instances.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 11, 2021
@ChrisCummins ChrisCummins force-pushed the fix/llvm-env-reset branch 2 times, most recently from b75e8db to 7a0a874 Compare March 11, 2021 15:10
@ChrisCummins ChrisCummins force-pushed the fix/llvm-env-reset branch 2 times, most recently from e00e62b to 64cb9d6 Compare March 13, 2021 01:19
@JD-ETH
Copy link
Contributor

JD-ETH commented Mar 13, 2021

Looks fine to me, but do you know what's failing the tests?

Copy link
Contributor

@JD-ETH JD-ETH left a comment

Choose a reason for hiding this comment

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

LGTM when the tests pass.

@ChrisCummins
Copy link
Contributor Author

Thanks @JD-ETH. I bumped cBench-v0 to cBench-v1 but didn't update this test. Will do that now

A regression introduced in:

    commit 6cccf05
    Author: Chris Cummins <chrisc.101@gmail.com>
    Date:   Fri Feb 26 14:10:35 2021 +0000

    [llvm] Install cBench-v0 if no benchmarks are installed.

means that env.reset() would not return an observation value for
LlvmEnv instances.
@ChrisCummins ChrisCummins merged commit f40799c into development Mar 15, 2021
@ChrisCummins ChrisCummins deleted the fix/llvm-env-reset branch March 15, 2021 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants