Refactor out Env interface from CompilerEnv #633

sogartar · 2022-03-19T01:17:26Z

The goal of this change is to have a clean separation between interface and implementation. It also allows for new environment implementations with approaches different from CompilerEnv.

codecov-commenter · 2022-03-19T01:42:04Z

Codecov Report

Merging #633 (3b9518a) into development (e6130db) will decrease coverage by 0.53%.
The diff coverage is 86.29%.

@@               Coverage Diff               @@
##           development     #633      +/-   ##
===============================================
- Coverage        88.48%   87.94%   -0.54%     
===============================================
  Files              115      116       +1     
  Lines             7066     7372     +306     
===============================================
+ Hits              6252     6483     +231     
- Misses             814      889      +75

Impacted Files	Coverage Δ
compiler_gym/service/compilation_session.py	`86.36% <ø> (ø)`
compiler_gym/envs/compiler_env.py	`75.79% <69.42%> (-14.47%)`	⬇️
compiler_gym/wrappers/core.py	`81.71% <84.25%> (-1.62%)`	⬇️
...ompiler_gym/service/client_service_compiler_env.py	`90.55% <90.55%> (ø)`
compiler_gym/envs/gcc/gcc_env.py	`100.00% <100.00%> (ø)`
compiler_gym/envs/llvm/llvm_env.py	`88.18% <100.00%> (ø)`
compiler_gym/envs/loop_tool/loop_tool_env.py	`75.00% <100.00%> (ø)`
compiler_gym/wrappers/commandline.py	`93.65% <100.00%> (+1.65%)`	⬆️
...er_gym/third_party/inst2vec/inst2vec_preprocess.py	`69.73% <0.00%> (-4.22%)`	⬇️
compiler_gym/util/commands.py	`83.33% <0.00%> (-2.39%)`	⬇️
... and 5 more

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 e6130db...3b9518a. Read the comment docs.

ChrisCummins

I'm still not 100% sure what the use case is for splitting interface / implementation like this, but I guess there's no harm in it.

However, I think the naming needs rethinking. Env implies that it isn't specific to compilers, which I believe it is, so I think the two choices are either:

Pick a name that makes it clear it is describing the compiler env, like CompilerEnvInterface or CompilerEnvBase.
Call it CompilerEnv and rename the current CompilerEnv to make it clear that it is an implementation of CompilerEnv, like ClientServiceCompilerEnv or even just CompilerEnvImplementation.

I think option 2 makes more sense to me. That way, all of the type hints for CompilerEnv now refer to the abstract interface. This may need updating code that refers to CompilerEnv and handles implementation details like interacting with env.service.

What do you think?

Cheers,
Chris

sogartar · 2022-03-21T15:03:19Z

I'm still not 100% sure what the use case is for splitting interface / implementation like this, but I guess there's no harm in it.

The idea is to follow the dependency inversion principle and open–closed principle. Users that write algorithms against this library would know to use only this interface and it will be stable upon changing the implementation. It may not be planed currently to have a different compiler service, but this leaves the door open. It makes clear to someone else who wants to make an environment what is the required interface to work with the algorithms.

However, I think the naming needs rethinking.

My intention was to not introduce a breaking change and since Env is in the compiler_gym package it should be clear that it is about compilers. Renaming the existing CompilerEnv class would be a breaking change for someone who extends it.
Aside form the breaking change the best option would be CompilerEnv for the interface and ClientServiceCompilerEnv for the implementation. Would you like me to use these names?

ChrisCummins · 2022-03-21T16:03:55Z

Thanks @sogartar!

It may not be planed currently to have a different compiler service, but this leaves the door open.

Yeah, I don't know if there's demand for this. But, as I said, I don't think it'll hurt, and it does seem like better practice to clearly delineate interface/implementation 🙂

Renaming the existing CompilerEnv class would be a breaking change for someone who extends it. Aside form the breaking change the best option would be CompilerEnv for the interface and ClientServiceCompilerEnv for the implementation. Would you like me to use these names?

Yes, I think this is worth a breaking change. Make CompilerEnv in compiler_env.py the interface, and rename the current CompilerEnv to a name that describes the implementation mechanism. I think ClientServiceCompilerEnv in client_service_compiler_env.py is a good choice for this, though I'm open to suggestions.

Cheers,
Chris

sogartar · 2022-03-22T19:41:58Z

@ChrisCummins, I made the renaming. Can you let me know what else should be included or removed in the interface? I don't think I can make the call. I am not sure that the observation_space_spec property should be a part or the benchmark or datasets related stuff.
I am also not sure of all the places were the interface should be used or the service. It really depends what should be in the interface.
The basic structure should be:

CompilerEnv <- ClientServiceCompilerEnv <- LllvmEnv, GccEnv, etc.
      ^
      |
Algorithms

Ideally, general algorithms should depend only on CompilerEnv.
I feel like the service client may be another important concept separate from the CompilerEnv interface and the gRPC specifics. It should define common service functionality, but that is a discussion for another time.

ChrisCummins · 2022-03-23T12:04:05Z

compiler_gym/envs/client_service_compiler_env.py

+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+


Needs a module docstring saying that this module defines an implementation of CompilerEnv interface using client/server approach

ChrisCummins · 2022-03-23T12:05:35Z

compiler_gym/envs/client_service_compiler_env.py

+
+
+class ClientServiceCompilerEnv(CompilerEnv):
+    """An OpenAI gym environment for compiler optimizations.


This docstring should be on the CompilerEnv base class

I moved a part of it to CompilerEnv.

ChrisCummins · 2022-03-23T12:07:03Z

compiler_gym/envs/client_service_compiler_env.py

+
+        :raises TimeoutError: If the compiler service fails to initialize within
+            the parameters provided in :code:`connection_settings`.
+        """


Does this need to invoke gym.Env superclass constructor?

Neither CompilerEnv or gym.Env have __init__(...).

compiler_gym/envs/compiler_env.py

ChrisCummins · 2022-03-23T13:12:20Z

@ChrisCummins, I made the renaming. Can you let me know what else should be included or removed in the interface? I don't think I can make the call. I am not sure that the observation_space_spec property should be a part or the benchmark or datasets related stuff. I am also not sure of all the places were the interface should be used or the service. It really depends what should be in the interface. The basic structure should be:
CompilerEnv <- ClientServiceCompilerEnv <- LllvmEnv, GccEnv, etc.
      ^
      |
Algorithms

Thanks for the update @sogartar. IMO everything that isn't solely relevant to the client/server design should be part of the CompilerEnv interface. Basically, extend "Algorithms" in your above diagram to "All user code".

This would make ClientServiceCompilerEnv an implementation detail that can be ignored by most users, and the only places where CompilerEnv needs updating to ClientServiceCompilerEnv is code which directly interacts with env.service or env.raw_step() (a handful of unit tests). The current CompilerEnv interface with only step/multistep/fork would require massive refactors to replace CompilerEnv with ClientServiceCompilerEnv, which seems like it would be going against the goal of separating interface from implementation, as 99.9% of the time no one cares if its client/server. Sorry if I wasn't clear on this, and I hope that your rename was an auto find+replace so it isn't a hassle to revert 😄

Ideally, general algorithms should depend only on CompilerEnv. I feel like the service client may be another important concept separate from the CompilerEnv interface and the gRPC specifics. It should define common service functionality, but that is a discussion for another time.

I wondered about whether it should be GrpcClientServiceCompilerEnv, as you could in theory have a different client/server architecture that doesn't use subprocesses + gRPC, but I think thats a bridge to cross when needed. It would require larger refactor to abstract the idea of a "service" from gRPC + protos.

One that could be worth doing now to highlight that ClientServiceCompilerEnv is an implementation detail would be to move the file from compiler_gym/envs/client_service_compiler_env.py into compiler_gym/service/client_service_compiler_env.py. That would then group all of the code relating to protos/grpc in compiler_gym/service.

Another question is whether CompilerEnv needs to be abstract. E.g. take a property like:

    @property
    @abstractmethod
    def episode_walltime(self) -> float:
        raise NotImplementedError("abstract method")

this seems like it would be straightforward enough bit of housekeeping for CompilerEnv to provide as it clearly don't depend on the implementation details of the environment. But that's always something that can be done later and isn't blocking for this PR.

Cheers,
Chris

sogartar · 2022-03-23T14:25:15Z

Some of the details are not properly isolated. For example ObservationSpaceSpec and Benchmark has protobuf related stuff in their interfaces. I can add to CompilerEnv the related functionality with the knowledge that this would break the isolation and then at a later stage fix them as well.

ChrisCummins · 2022-03-23T15:49:49Z

Yeah, I think they are candidates for future refactoring, but we unfortunately can't get away from dropping them completely from the CompilerEnv interface without having to update a lot of code.

Cheers,
Chris

sogartar · 2022-03-24T01:30:21Z

@ChrisCummins, I think this PR is ready for another code-review. I did what you suggested. One thing to note is that moving ClientServiceCompilerEnv to service caused a circular dependency that I broke by not making ClientServiceCompilerEnv a dependency of the service package. I think this will auto-resolve if benchmark, dataset, etc. do not depend on service.

sogartar · 2022-03-24T15:49:12Z

I am not sure why this failure https://github.com/facebookresearch/CompilerGym/runs/5669835818?check_suite_focus=true#step:8:213 started appearing now. It should have triggered before also, since gym.Wrapper redirects each attribute to the underlying env. What I did is to add the property explicitly in CompilerEnvWrapper for better stack tracing.

ChrisCummins · 2022-03-24T18:11:07Z

I am not sure why this failure https://github.com/facebookresearch/CompilerGym/runs/5669835818?check_suite_focus=true#step:8:213 started appearing now. It should have triggered before also, since gym.Wrapper redirects each attribute to the underlying env. What I did is to add the property explicitly in CompilerEnvWrapper for better stack tracing.

Is the problem the UserWarning?

ChrisCummins

Hi @sogartar, this is coming along great! I've come around to like the separation of interface / gRPC implementation and am already thinking of environments I'd like to implement that aren't based on this client / service design 🙂

Last few nitpicky requests left inline.

Also, I think there's a handful of files where the class name has been updated where it doesn't need to be. Please revert them (unless I'm wrong!):

benchmarks/bench_test.py
compiler_gym/bin/service.py
compiler_gym/envs/llvm/llvm_benchmark.py
compiler_gym/random_replay.py
compiler_gym/util/debug_util.py
examples/llvm_autotuning/autotuners/opentuner_.py
examples/llvm_rl/model/inference_result.py
leaderboard/llvm_instcount/random_search/README.md

Cheers,
Chris

ChrisCummins · 2022-03-24T18:15:42Z

compiler_gym/envs/compiler_env.py

-
-
-class CompilerEnv(gym.Env):
+class CompilerEnv(gym.Env, ABC):


I think there are still a few properties missing from this interface: version, compiler_version (possibly others)

I added them and a few more.

ChrisCummins · 2022-03-24T18:19:37Z

compiler_gym/envs/compiler_env.py

-    def render(
-        self,
-        mode="human",
-    ) -> Optional[str]:
-        """Render the environment.
-
-        CompilerEnv instances support two render modes: "human", which prints
-        the current environment state to the terminal and return nothing; and
-        "ansi", which returns a string representation of the current environment
-        state.
-
-        :param mode: The render mode to use.
-        :raises TypeError: If a default observation space is not set, or if the
-            requested render mode does not exist.
        """


I think this should be on CompilerEnv. Is the reason it isn't because it is also on gym.Env?

The reason was that it is a part of gym.Env, but I added it in CompilerEnv as well. It has a type hint and the additional documentation.

ChrisCummins · 2022-03-24T18:22:32Z

compiler_gym/random_replay.py

@@ -8,7 +8,7 @@

 from deprecated import deprecated

-from compiler_gym.envs.compiler_env import CompilerEnv


IRRC, this is actually useful. It means that //compiler_gym:random_replay need only depend on //compiler_gym/envs:compiler_env rather than //compiler_gym

ChrisCummins · 2022-03-24T18:23:07Z

compiler_gym/service/BUILD

+        # TODO: add this after circular dependencies are resolved
+        # ":client_service_compiler_env",


I wonder if the easies thing here would be to merge :service and :client_service_compiler_env, so targets need only depend on //compiler_gym/service?

(same for cmake)

This will cause circular dependency. I have more elaborate explanation in another comment.

Okay, in that case could you update the TODO note to reference this?

TODO(github.com/facebookresearch/CompilerGym/pull/633): ...

ChrisCummins · 2022-03-24T18:24:33Z

compiler_gym/service/client_service_compiler_env.py

+    :ivar action_spaces: A list of supported action space names.
+
+    :vartype action_spaces: List[str]
+
+    :ivar actions: The list of actions that have been performed since the
+        previous call to :func:`reset`.
+
+    :vartype actions: List[ActionType]
+
+    :ivar reward_range: A tuple indicating the range of reward values. Default
+        range is (-inf, +inf).
+
+    :vartype reward_range: Tuple[float, float]
+
+    :ivar observation: A view of the available observation spaces that permits
+        on-demand computation of observations.
+
+    :vartype observation: compiler_gym.views.ObservationView
+
+    :ivar reward: A view of the available reward spaces that permits on-demand
+        computation of rewards.
+
+    :vartype reward: compiler_gym.views.RewardView
+
+    :ivar episode_reward: If :func:`ClientServiceCompilerEnv.reward_space
+        <compiler_gym.envs.CompilerGym.reward_space>` is set, this value is the
+        sum of all rewards for the current episode.
+
+    :vartype episode_reward: float


I think each of these ivar comments should a docstring on the corresponding abstract property in CompilerEnv.

ChrisCummins · 2022-03-24T18:28:13Z

examples/example_compiler_gym_service/__init__.py

@@ -83,7 +83,7 @@ def benchmark_from_parsed_uri(self, uri: BenchmarkUri) -> Benchmark:
 # the example-v0 environment will be available to gym.make(...).
 register(
    id="example-cc-v0",
-    entry_point="compiler_gym.envs:CompilerEnv",
+    entry_point="compiler_gym.service.client_service_compiler_env:ClientServiceCompilerEnv",


It would be nice to add ClientServiceCompilerEnv to the __all__ list in compiler_gym/service/__init__.py, so this entry point would be compiler_gym.service:ClientServiceCopmilerEnv.

and for the others in this file

This would cause circular import. I tried it already. I am not sure how to resolve the issue. The problem is that stuff in dataset and view depend on service and by doing this service would depend on dataset and view. The correct solution would be to also split dataset and view into interface and implementation and have ClientServiceCopmilerEnv depend on the interfaces.

If ClientServiceCopmilerEnv needs the details, they would be implementations related to the gRPC service, so they would also be able to move to service.

Ah I see, thanks for the context. Okay, let's keep it the way you have done. Long term, I think it makes to break dataset and view dependencies on service. Maybe through an equivalent refactor between the dataset/view "interface" and gRPC implementation?

ChrisCummins · 2022-03-24T18:29:13Z

examples/loop_optimizations_service/__init__.py

@@ -137,7 +137,7 @@ def benchmark_from_parsed_uri(self, uri: BenchmarkUri) -> Benchmark:

 register(
    id="loops-opt-py-v0",
-    entry_point="compiler_gym.envs:CompilerEnv",
+    entry_point="compiler_gym.service.client_service_compiler_env:ClientServiceCompilerEnv",


Suggest compiler_gym.service:ClientServiceCompilerEnv

See my other comments about circular dependencies.

ChrisCummins · 2022-03-24T18:32:44Z

Oh, and the new ClientServiceCompilerEnv needs to be added to the documentation. I'd suggest this page https://compilergym.com/compiler_gym/service.html

sogartar · 2022-03-25T14:59:01Z

I am not sure why this failure https://github.com/facebookresearch/CompilerGym/runs/5669835818?check_suite_focus=true#step:8:213 started appearing now. It should have triggered before also, since gym.Wrapper redirects each attribute to the underlying env. What I did is to add the property explicitly in CompilerEnvWrapper for better stack tracing.

Is the problem the UserWarning?

It seems it is. The test should probably expect the warning.

sogartar · 2022-03-25T19:31:14Z

Oh, and the new ClientServiceCompilerEnv needs to be added to the documentation. I'd suggest this page https://compilergym.com/compiler_gym/service.html

I added it there, but technically ClientServiceCompilerEnv is not a part of service due to the circular dependency.

sogartar · 2022-03-25T19:32:33Z

Also, I think there's a handful of files where the class name has been updated where it doesn't need to be. Please revert them (unless I'm wrong!):
benchmarks/bench_test.py
compiler_gym/bin/service.py
compiler_gym/envs/llvm/llvm_benchmark.py
compiler_gym/random_replay.py
compiler_gym/util/debug_util.py
examples/llvm_autotuning/autotuners/opentuner_.py
examples/llvm_rl/model/inference_result.py
leaderboard/llvm_instcount/random_search/README.md

I renamed most of them.
I think these need ClientServiceCompilerEnv:

benchmarks/bench_test.py
examples/llvm_autotuning/autotuners/opentuner_.py

They use stuff like send_param and service.connection

sogartar · 2022-03-25T19:44:49Z

The PR is ready of another review.

sogartar · 2022-03-26T01:34:38Z

@ChrisCummins, do you have an idea why the pre-commit checks are failing?

sogartar · 2022-03-26T01:40:34Z

One thing to mention is that the ObservationWrapper and RewardWrapper needed to have their conversion methods renamed because they conflict with methods from the CompilerEnv base. These are observation(...) and reward(...).

ChrisCummins · 2022-03-27T10:04:39Z

@ChrisCummins, do you have an idea why the pre-commit checks are failing?

No, sorry. Looks like its a general error, not specific to these changes. Does development branch fail?

I think these need ClientServiceCompilerEnv:
...
They use stuff like send_param and service.connection

Good catch!

ChrisCummins

Looking good. There are a couple of CI failures that need addressing prior to merge.

I think the observation() to convert_observation() rename makes sense

Cheers,
Chris

ChrisCummins · 2022-03-27T10:08:17Z

compiler_gym/service/BUILD

+        # TODO: add this after circular dependencies are resolved
+        # ":client_service_compiler_env",


Okay, in that case could you update the TODO note to reference this?

TODO(github.com/facebookresearch/CompilerGym/pull/633): ...

ChrisCummins · 2022-03-27T10:11:11Z

examples/example_compiler_gym_service/__init__.py

@@ -83,7 +83,7 @@ def benchmark_from_parsed_uri(self, uri: BenchmarkUri) -> Benchmark:
 # the example-v0 environment will be available to gym.make(...).
 register(
    id="example-cc-v0",
-    entry_point="compiler_gym.envs:CompilerEnv",
+    entry_point="compiler_gym.service.client_service_compiler_env:ClientServiceCompilerEnv",


Ah I see, thanks for the context. Okay, let's keep it the way you have done. Long term, I think it makes to break dataset and view dependencies on service. Maybe through an equivalent refactor between the dataset/view "interface" and gRPC implementation?

ChrisCummins · 2022-03-27T10:12:09Z

tests/wrappers/core_wrappers_test.py

@@ -18,9 +19,30 @@
 pytest_plugins = ["tests.pytest_plugins.llvm"]


+class ObservationDummyWrapper(ObservationWrapper):


Why ObservationDummyWrapper and not ObservationWrapper (using an import .. as .. to avoid shadowing the ObservationWrapper in gym module)

ChrisCummins · 2022-03-27T10:13:49Z

tests/wrappers/core_wrappers_test.py

-
-    env = ObservationWrapper(env)
-    with pytest.raises(NotImplementedError):
-        env.reset()
+    with pytest.raises(TypeError):
+        env = ObservationWrapper(env)


Hmm, why is it now a TypeError? Could you add a match="..." argument to match a bit of the description?

I made the class ObservationWrapper an abstract base class, so if you miss to implement some abstract method it raises a TypeError. There is no match because I am not confident that the exception message string that ABC uses will be stable.

sogartar · 2022-03-28T15:40:32Z

Ah I see, thanks for the context. Okay, let's keep it the way you have done. Long term, I think it makes to break dataset and view dependencies on service. Maybe through an equivalent refactor between the dataset/view "interface" and gRPC implementation?

This approach should work.

sogartar · 2022-03-28T16:14:05Z

Okay, in that case could you update the TODO note to reference this?
TODO(github.com/facebookresearch/CompilerGym/pull/633): ...

Done.

sogartar · 2022-03-28T16:17:49Z

@ChrisCummins, do you have an idea why the pre-commit checks are failing?

No, sorry. Looks like its a general error, not specific to these changes. Does development branch fail?

It seems it does 6cf4926.

…apper

sogartar · 2022-03-28T19:10:20Z

@ChrisCummins, could you do another review round?

ChrisCummins · 2022-03-29T19:17:22Z

LGTM, thanks so much @sogartar!

@sogartar

This release adds a new compiler environment, new APIs, and a suite of backend improvements to improve the flexibility of CompilerGym environments. Many thanks to code contributors: @sogartar, @KyleHerndon, @SoumyajitKarmakar, @uduse, and @anthony0727! Highlights of this release include: - [mlir] Began work on a new environment for matrix multiplication using MLIR ([#652](#652), thanks @KyleHerndon and @sogartar!). Note this environment is not yet included in the pypi package and must be [compiled from source](https://github.com/facebookresearch/CompilerGym/blob/development/INSTALL.md#building-from-source-with-cmake). - [llvm] Added a new `env.benchmark_from_clang_invocation()` method ([#577](#577)) that can be used for constructing LLVM environment automatically from C/C++ compiler invocations. This makes it much easier to integrate CompilerGym with your existing build scripts. - Added three new wrapper classes: `Counter`, that provides op counts for analysis ([#683](#683)); `SynchronousSqliteLogger`, that provides logging of environment interactions to a relational database ([#679](#679)), and `ForkOnStep` that provides an `undo()` operation ([#682](#682)). - Added `reward_space` and `observation_space` parameters to `env.reset()` ([#659](#659), thanks @SoumyajitKarmakar!) This release includes a number of improvements to the backend APIs that make it easier to write new CompilerGym environments: - Refactored the backend to make `CompilerEnv` an abstract interface, and `ClientServiceCompilerEnv` the concrete implementation of this interface. This enables new environments to be implemented without using gRPC ([#633](#633), thanks @sogartar!). - Extended the support for different types of action and observation spaces ([#641](#641), [#643](#643), thanks @sogartar!), including new `Permutation` and `SpaceSequence` spaces ([#645](#645), thanks @sogartar!).. - Added a new `disk/` subdirectory to compiler service's working directories, which is symlinked to an on-disk location for devices which support in-memory working directories. This fixes a bug with leftover temporary directories from LLVM ([#672](#672)). This release also includes numerous bug fixes and improvements, many of which were reported or fixed by the community. For example, fixing a bug in cache file locations ([#656](#656), thanks @uduse!), and a missing flag definition in example code ([#684](#684), thanks @anthony0727!). **Full Changelog**: v0.2.3...v0.2.4 This release brings in deprecating changes to the core `env.step()` routine, and lays the groundwork for enabling new types of compiler optimizations to be exposed through CompilerGym. Many thanks to code contributors: @mostafaelhoushi, @sogartar, @KyleHerndon, @uduse, @parthchadha, and @xtremey! Highlights of this release include: - Added a new `TextSizeInBytes` observation space for LLVM ([#575](#575)). * Added a new PPO leaderboard entry ([#580](#580). Thanks @xtremey! - Fixed a bug in which temporary directories created by the LLVM environment were not cleaned up ([#592](#592)). - **[Backend]** The function `createAndRunCompilerGymService` now returns an int, which is the exit return code ([#592](#592)). - Improvements to the examples documentation ([#548](#548)) and FAQ ([#586](#586)) Deprecations and breaking changes: - `CompilerEnv.step` no longer accepts a list of actions ([#627](#627)). A new method, `CompilerEnv.multistep` provides this functionality. This is to provide compatibility with environments whose action spaces are lists. To update your code, replace any calls to `env.step()` which take a list of actions to use `env.multistep()`. Thanks @sogartar! - The arguments `observations` and `rewards` to `step()` have been renamed `observation_spaces` and `reward_spaces`, respectively ([#627](#627)). - `Reward.id` has been renamed `Reward.name` ([#565](#565), [#612](#612)). Thanks @parthchadha! * The backend protocol buffer schema has been updated to natively support more types of observation and action, and to support nested spaces ([#531](#531)). Thanks @sogartar!

@sogartar

This release adds a new compiler environment, new APIs, and a suite of backend improvements to improve the flexibility of CompilerGym environments. Many thanks to code contributors: @sogartar, @KyleHerndon, @SoumyajitKarmakar, @uduse, and @anthony0727! Highlights of this release include: - [mlir] Began work on a new environment for matrix multiplication using MLIR ([#652](#652), thanks @KyleHerndon and @sogartar!). Note this environment is not yet included in the pypi package and must be [compiled from source](https://github.com/facebookresearch/CompilerGym/blob/development/INSTALL.md#building-from-source-with-cmake). - [llvm] Added a new `env.benchmark_from_clang_invocation()` method ([#577](#577)) that can be used for constructing LLVM environment automatically from C/C++ compiler invocations. This makes it much easier to integrate CompilerGym with your existing build scripts. - Added three new wrapper classes: `Counter`, that provides op counts for analysis ([#683](#683)); `SynchronousSqliteLogger`, that provides logging of environment interactions to a relational database ([#679](#679)), and `ForkOnStep` that provides an `undo()` operation ([#682](#682)). - Added `reward_space` and `observation_space` parameters to `env.reset()` ([#659](#659), thanks @SoumyajitKarmakar!) This release includes a number of improvements to the backend APIs that make it easier to write new CompilerGym environments: - Refactored the backend to make `CompilerEnv` an abstract interface, and `ClientServiceCompilerEnv` the concrete implementation of this interface. This enables new environments to be implemented without using gRPC ([#633](#633), thanks @sogartar!). - Extended the support for different types of action and observation spaces ([#641](#641), [#643](#643), thanks @sogartar!), including new `Permutation` and `SpaceSequence` spaces ([#645](#645), thanks @sogartar!).. - Added a new `disk/` subdirectory to compiler service's working directories, which is symlinked to an on-disk location for devices which support in-memory working directories. This fixes a bug with leftover temporary directories from LLVM ([#672](#672)). This release also includes numerous bug fixes and improvements, many of which were reported or fixed by the community. For example, fixing a bug in cache file locations ([#656](#656), thanks @uduse!), and a missing flag definition in example code ([#684](#684), thanks @anthony0727!). **Full Changelog**: v0.2.3...v0.2.4 This release brings in deprecating changes to the core `env.step()` routine, and lays the groundwork for enabling new types of compiler optimizations to be exposed through CompilerGym. Many thanks to code contributors: @mostafaelhoushi, @sogartar, @KyleHerndon, @uduse, @parthchadha, and @xtremey! Highlights of this release include: - Added a new `TextSizeInBytes` observation space for LLVM ([#575](#575)). * Added a new PPO leaderboard entry ([#580](#580). Thanks @xtremey! - Fixed a bug in which temporary directories created by the LLVM environment were not cleaned up ([#592](#592)). - **[Backend]** The function `createAndRunCompilerGymService` now returns an int, which is the exit return code ([#592](#592)). - Improvements to the examples documentation ([#548](#548)) and FAQ ([#586](#586)) Deprecations and breaking changes: - `CompilerEnv.step` no longer accepts a list of actions ([#627](#627)). A new method, `CompilerEnv.multistep` provides this functionality. This is to provide compatibility with environments whose action spaces are lists. To update your code, replace any calls to `env.step()` which take a list of actions to use `env.multistep()`. Thanks @sogartar! - The arguments `observations` and `rewards` to `step()` have been renamed `observation_spaces` and `reward_spaces`, respectively ([#627](#627)). - `Reward.id` has been renamed `Reward.name` ([#565](#565), [#612](#612)). Thanks @parthchadha! * The backend protocol buffer schema has been updated to natively support more types of observation and action, and to support nested spaces ([#531](#531)). Thanks @sogartar!

@sogartar

This release adds a new compiler environment, new APIs, and a suite of backend improvements to improve the flexibility of CompilerGym environments. Many thanks to code contributors: @sogartar, @KyleHerndon, @SoumyajitKarmakar, @uduse, and @anthony0727! Highlights of this release include: - [mlir] Began work on a new environment for matrix multiplication using MLIR ([#652](#652), thanks @KyleHerndon and @sogartar!). Note this environment is not yet included in the pypi package and must be [compiled from source](https://github.com/facebookresearch/CompilerGym/blob/development/INSTALL.md#building-from-source-with-cmake). - [llvm] Added a new `env.benchmark_from_clang_invocation()` method ([#577](#577)) that can be used for constructing LLVM environment automatically from C/C++ compiler invocations. This makes it much easier to integrate CompilerGym with your existing build scripts. - Added three new wrapper classes: `Counter`, that provides op counts for analysis ([#683](#683)); `SynchronousSqliteLogger`, that provides logging of environment interactions to a relational database ([#679](#679)), and `ForkOnStep` that provides an `undo()` operation ([#682](#682)). - Added `reward_space` and `observation_space` parameters to `env.reset()` ([#659](#659), thanks @SoumyajitKarmakar!) This release includes a number of improvements to the backend APIs that make it easier to write new CompilerGym environments: - Refactored the backend to make `CompilerEnv` an abstract interface, and `ClientServiceCompilerEnv` the concrete implementation of this interface. This enables new environments to be implemented without using gRPC ([#633](#633), thanks @sogartar!). - Extended the support for different types of action and observation spaces ([#641](#641), [#643](#643), thanks @sogartar!), including new `Permutation` and `SpaceSequence` spaces ([#645](#645), thanks @sogartar!).. - Added a new `disk/` subdirectory to compiler service's working directories, which is symlinked to an on-disk location for devices which support in-memory working directories. This fixes a bug with leftover temporary directories from LLVM ([#672](#672)). This release also includes numerous bug fixes and improvements, many of which were reported or fixed by the community. For example, fixing a bug in cache file locations ([#656](#656), thanks @uduse!), and a missing flag definition in example code ([#684](#684), thanks @anthony0727!). **Full Changelog**: v0.2.3...v0.2.4 This release brings in deprecating changes to the core `env.step()` routine, and lays the groundwork for enabling new types of compiler optimizations to be exposed through CompilerGym. Many thanks to code contributors: @mostafaelhoushi, @sogartar, @KyleHerndon, @uduse, @parthchadha, and @xtremey! Highlights of this release include: - Added a new `TextSizeInBytes` observation space for LLVM ([#575](#575)). * Added a new PPO leaderboard entry ([#580](#580). Thanks @xtremey! - Fixed a bug in which temporary directories created by the LLVM environment were not cleaned up ([#592](#592)). - **[Backend]** The function `createAndRunCompilerGymService` now returns an int, which is the exit return code ([#592](#592)). - Improvements to the examples documentation ([#548](#548)) and FAQ ([#586](#586)) Deprecations and breaking changes: - `CompilerEnv.step` no longer accepts a list of actions ([#627](#627)). A new method, `CompilerEnv.multistep` provides this functionality. This is to provide compatibility with environments whose action spaces are lists. To update your code, replace any calls to `env.step()` which take a list of actions to use `env.multistep()`. Thanks @sogartar! - The arguments `observations` and `rewards` to `step()` have been renamed `observation_spaces` and `reward_spaces`, respectively ([#627](#627)). - `Reward.id` has been renamed `Reward.name` ([#565](#565), [#612](#612)). Thanks @parthchadha! * The backend protocol buffer schema has been updated to natively support more types of observation and action, and to support nested spaces ([#531](#531)). Thanks @sogartar!

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 19, 2022

ChrisCummins reviewed Mar 21, 2022

View reviewed changes

sogartar added 3 commits March 22, 2022 08:26

Refactor out Env interface from CompilerEnv

e785540

Rename CompilerEnv to ClientServiceCompilerEnv

ecf42fb

Rename Env to CompilerEnv

4a79017

sogartar force-pushed the refactor-out-env-iface branch from 72e7740 to 4a79017 Compare March 22, 2022 19:25

ChrisCummins reviewed Mar 23, 2022

View reviewed changes

compiler_gym/envs/compiler_env.py Show resolved Hide resolved

sogartar added 4 commits March 23, 2022 14:35

Extend CompilerEnv interface

1e6595e

Rename back some occurrences of ClientServiceCompilerEnv to CompilerEnv

d667d7f

Move ClientServiceCompilerEnv to service

f760d42

Edit documentation for CompilerEnv and ClientServiceCompilerEnv

621cace

Remove duplicate benchmark property from ClientServiceCompilerEnv

2acd700

ChrisCummins reviewed Mar 24, 2022

View reviewed changes

sogartar added 2 commits March 25, 2022 10:08

Add missing methods to CompilerEnv

2b041d6

Rename in a few places ClientServiceCompilerEnv to CompilerEnv

b7f0ac0

sogartar added 2 commits March 25, 2022 12:22

Add ClientServiceCompilerEnv to documentation

33a17cc

Remove comments of some members in ClientServiceCompilerEnv

4715691

Add expected warning in test_wrapped_set_benchmark

64f3ef2

sogartar force-pushed the refactor-out-env-iface branch from 1264556 to 64f3ef2 Compare March 26, 2022 00:12

Use convert_observation in ConcatActionsHistogram

4230f9f

ChrisCummins reviewed Mar 27, 2022

View reviewed changes

Add reference to PR in a comment about circular dependency

165b385

sogartar force-pushed the refactor-out-env-iface branch from c3dc175 to 26af19e Compare March 28, 2022 16:13

Fix implementation of inheritace from ObservationWrapper and RewardWr…

3b9518a

…apper

sogartar force-pushed the refactor-out-env-iface branch from 26af19e to 3b9518a Compare March 28, 2022 16:27

ChrisCummins merged commit 3c21aee into facebookresearch:development Mar 29, 2022

ChrisCummins mentioned this pull request May 24, 2022

CompilerGym v0.2.4 #686

Merged

ChrisCummins mentioned this pull request May 25, 2022

CompilerGym v0.2.4 #689

Merged



		class ClientServiceCompilerEnv(CompilerEnv):
		"""An OpenAI gym environment for compiler optimizations.

		@@ -8,7 +8,7 @@

		from deprecated import deprecated

		from compiler_gym.envs.compiler_env import CompilerEnv

		# TODO: add this after circular dependencies are resolved
		# ":client_service_compiler_env",

		@@ -18,9 +19,30 @@
		pytest_plugins = ["tests.pytest_plugins.llvm"]


		class ObservationDummyWrapper(ObservationWrapper):

Refactor out Env interface from CompilerEnv #633

Refactor out Env interface from CompilerEnv #633

Conversation

sogartar commented Mar 19, 2022

codecov-commenter commented Mar 19, 2022 • edited Loading

Codecov Report

ChrisCummins left a comment

Choose a reason for hiding this comment

sogartar commented Mar 21, 2022 • edited Loading

ChrisCummins commented Mar 21, 2022

sogartar commented Mar 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisCummins commented Mar 23, 2022

sogartar commented Mar 23, 2022

ChrisCummins commented Mar 23, 2022

sogartar commented Mar 24, 2022

sogartar commented Mar 24, 2022

ChrisCummins commented Mar 24, 2022

ChrisCummins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sogartar Mar 25, 2022 • edited Loading

Choose a reason for hiding this comment

sogartar Mar 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisCummins commented Mar 24, 2022

sogartar commented Mar 25, 2022

sogartar commented Mar 25, 2022

sogartar commented Mar 25, 2022

sogartar commented Mar 25, 2022

sogartar commented Mar 26, 2022

sogartar commented Mar 26, 2022 • edited Loading

ChrisCummins commented Mar 27, 2022

ChrisCummins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sogartar commented Mar 28, 2022

sogartar commented Mar 28, 2022

sogartar commented Mar 28, 2022

sogartar commented Mar 28, 2022 • edited Loading

ChrisCummins commented Mar 29, 2022

codecov-commenter commented Mar 19, 2022 •

edited

Loading

sogartar commented Mar 21, 2022 •

edited

Loading

sogartar Mar 25, 2022 •

edited

Loading

sogartar Mar 25, 2022 •

edited

Loading

sogartar commented Mar 26, 2022 •

edited

Loading

sogartar commented Mar 28, 2022 •

edited

Loading