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

Add Observations to Loop Optimizations Environment #570

Conversation

mostafaelhoushi
Copy link
Contributor

@mostafaelhoushi mostafaelhoushi commented Feb 14, 2022

  • Adds inst2vec [Done], Autophase [Done], and Programl [In Progress] to the Loop Optimizations Environment
  • to enable extracting ProGraml I created a small compute_programl.cc program. I tried first to place it in external/programl but was getting build errors, so I temporarily placed it in the loop opt example directory.

Issues:

  • we need to first buld compute_autophase separately before we can run our example:
bazel build //compiler_gym/third_party/autophase:compute_autophase-prelinked

bazel run //examples/loop_optimizations_service:example

TODO (before merging):

  • fix build scripts so that we won't need to build compute_autophase
  • move compute_programl to a suitable directory
  • complete Programl observation
  • add unit test cases for each observation
  • add AutoPhase dictionary observation
  • verify build with CMake on Linux

@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 Feb 14, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2022

Codecov Report

Merging #570 (7d42155) into development (0394e60) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #570      +/-   ##
===============================================
+ Coverage        88.17%   88.20%   +0.02%     
===============================================
  Files              114      114              
  Lines             6708     6708              
===============================================
+ Hits              5915     5917       +2     
+ Misses             793      791       -2     
Impacted Files Coverage Δ
compiler_gym/service/proto/__init__.py 100.00% <ø> (ø)
...loop_tool/service/loop_tool_compilation_session.py 88.43% <0.00%> (-1.37%) ⬇️
compiler_gym/envs/compiler_env.py 91.83% <0.00%> (+0.49%) ⬆️
compiler_gym/views/observation.py 100.00% <0.00%> (+2.70%) ⬆️
compiler_gym/views/observation_space_spec.py 89.28% <0.00%> (+3.57%) ⬆️

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 0394e60...7d42155. Read the comment docs.

@mostafaelhoushi
Copy link
Contributor Author

@ChrisCummins, I addressed the comments and enabled Programl observation (also moved compute_programl to compiler_gym/third_party/programl.

However, I am currently printing programl observation as a Json string. Is that how the originl CompilerGym LLVM environments print Programl observation?

@ChrisCummins
Copy link
Contributor

@ChrisCummins, I addressed the comments and enabled Programl observation (also moved compute_programl to compiler_gym/third_party/programl.

Thanks! And yep, compiler_gym/third_party sounds like a good idea.

However, I am currently printing programl observation as a Json string. Is that how the originl CompilerGym LLVM environments print Programl observation?

This is exactly what the LLVM environment does:

// Serialize the graph to a JSON node link graph.
json nodeLinkGraph;
status = programl::graph::format::ProgramGraphToNodeLinkGraph(graph, &nodeLinkGraph);
if (!status.ok()) {
return Status(StatusCode::INTERNAL, status.error_message());
}
Opaque opaque;
opaque.set_format(space == LlvmObservationSpace::PROGRAML ? "json://networkx/MultiDiGraph"
: "json://");
*opaque.mutable_data() = nodeLinkGraph.dump();
reply.mutable_any_value()->PackFrom(opaque);

However, since merging #531, this could be made more efficient for the LLVM environment by embedding the ProgramGraph protocol buffer directly in the observation, removing the need to do a JSON serialize/deserialize. I haven't got around to doing that refactor yet though.

Cheers,
Chris

Copy link
Contributor

@ChrisCummins ChrisCummins left a comment

Choose a reason for hiding this comment

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

I agree with your TODO list in the PR message about adding some extra test cases to cover these new observations. Otherwise, LGTM!

@mostafaelhoushi mostafaelhoushi marked this pull request as ready for review February 26, 2022 23:50
@mostafaelhoushi mostafaelhoushi changed the title WIP: Add Observations to Loop Optimizations Environment Add Observations to Loop Optimizations Environment Feb 26, 2022
@mostafaelhoushi mostafaelhoushi merged commit d5d60b6 into facebookresearch:development Mar 4, 2022
Comment on lines +15 to +16
compiler_gym::third_party::autophase:compute_autophase
compiler_gym::third_party::programl:compute_programl
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Building on Linux is failing as I get this error (see CI log here):

-- Configuring done
CMake Error at build_tools/cmake/cg_macros.cmake:281 (add_dependencies):
  The dependency target
  "compiler_gym__third_party__autophase_compute_autophase" of target
  "examples__loop_optimizations_service__service_py__loops_opt_service" does
  not exist.
Call Stack (most recent call first):
  build_tools/cmake/cg_py_library.cmake:78 (cg_add_data_dependencies)
  examples/loop_optimizations_service/service_py/CMakeLists.txt:8 (cg_py_library)


CMake Error at build_tools/cmake/cg_macros.cmake:281 (add_dependencies):
  The dependency target
  "compiler_gym__third_party__programl_compute_programl" of target
  "examples__loop_optimizations_service__service_py__loops_opt_service" does
  not exist.
Call Stack (most recent call first):
  build_tools/cmake/cg_py_library.cmake:78 (cg_add_data_dependencies)
  examples/loop_optimizations_service/service_py/CMakeLists.txt:8 (cg_py_library)


-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.
Error: Process completed with exit code 1.

I tried many times to fix it, and with the issue that CMake configuration takes multiple hours (see #595 ), it was really difficult to solve it. So I merged for now and will fix it later.

Cc @ChrisCummins @sogartar

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.

5 participants