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

[examples] Update and improve the example service implementations and documentation #467

Merged
merged 9 commits into from
Oct 14, 2021

Conversation

ChrisCummins
Copy link
Contributor

This patch set includes:

  • updates and corrects the README file for the example gym services, fixing How to run the Example CompilerGym Service #466.
  • this adds a new demo.py script that includes a minimal usage of the example environments. This is intended to be used as a starting point for people who write their own services.
  • adds a new demo_without_bazel.py script which shows how the Python service can be used independently of the bazel build system.
  • fixes a bug in which --port flag was not used by the Python service runtime.
  • fixes a minor behavior difference between the C / Python runtimes in that C required the logs directory to exist wheras the Python version would create the directory. Both runtimes now create the directory.

@ChrisCummins ChrisCummins added Documentation Improvements or additions to documentation Enhancement New feature or request labels Oct 13, 2021
@ChrisCummins ChrisCummins added this to the v0.2.1 milestone Oct 13, 2021
@mostafaelhoushi
Copy link
Contributor

LGTM
Wondering if demo.py and demo_without_bazel.py can be added to CI? or added to env_tests.py?

@ChrisCummins
Copy link
Contributor Author

Thanks for the quick response @mostafaelhoushi!

Wondering if demo.py and demo_without_bazel.py can be added to CI? or added to env_tests.py?

demo_without_bazel.py will be tested as part of the CI workflow in the "examples-test" job. Unfortunately demo.py (and env_tests.py) are not tested in the CI because they are bazel-only :( Long term I will try and de-bazel the example service but that will depend on figuring out a way to build the C service without bazel.

Cheers,
Chris

@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 Oct 13, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2021

Codecov Report

Merging #467 (ee4e19d) into development (33529fc) will increase coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #467       /-   ##
===============================================
  Coverage        87.55%   87.63%    0.08%     
===============================================
  Files              109      109              
  Lines             6113     6113              
===============================================
  Hits              5352     5357        5     
  Misses             761      756       -5     
Impacted Files Coverage Δ
...ice/runtime/create_and_run_compiler_gym_service.py 100.00% <ø> (ø)
...loop_tool/service/loop_tool_compilation_session.py 89.26% <0.00%> (ø)
compiler_gym/envs/llvm/datasets/cbench.py 77.44% <0.00%> ( 0.75%) ⬆️
compiler_gym/service/connection.py 78.04% <0.00%> ( 1.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33529fc...ee4e19d. Read the comment docs.

@ChrisCummins ChrisCummins merged commit 6fe92d6 into facebookresearch:development Oct 14, 2021
@ChrisCummins ChrisCummins deleted the examples branch October 14, 2021 09:08
This was referenced Nov 17, 2021
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. Documentation Improvements or additions to documentation Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants