From a6859d6a4e08870c7b3fd89572ba1cb73f114545 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 13 Sep 2024 08:21:49 -0700 Subject: [PATCH 1/6] Restrict push CI to main, fix permissions (#363) This patch makes two primary changes to the CI config. It restricts the CI running on push events to only run on the main branch. This prevents the CI from running twice on PRs from branches in the main repository, which are needed for stacked PRs. Additionally, this PR sets the permissions at the top of the file, which is a general best practice for Github workflows for security reasons. Additionally, bump a couple of the actions that were using deprecated NodeJS versions so we do not get the warning anymore. --- .github/workflows/main.yml | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b4fece12..71a8605c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -2,13 +2,21 @@ name: MLGO CI -on: [push, repository_dispatch, pull_request] +permissions: + contents: read + +on: + push: + branches: + - 'main' + repository_dispatch: + pull_request: jobs: LicenseCheck: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: ./check-license.sh Envvars: runs-on: ubuntu-latest @@ -46,17 +54,17 @@ jobs: - task: Test cmd: pytest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Install Python With Cached pip Packages if: needs.Envvars.outputs.do_cache == '1' - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} cache: 'pipenv' cache-dependency-path: Pipfile.lock - name: Install Python, no cache if: needs.Envvars.outputs.do_cache == '0' - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Install Pipenv From 91c407c0af2b5f8061f0b690dd33e48412218aaa Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 13 Sep 2024 10:55:44 -0700 Subject: [PATCH 2/6] Delete unused ESWorker (#365) This patch deletes es_worker.py, which is not used anywhere, even for testing. An exact copy of the class is in blackbox_learner_test.py for testing purposes. --- compiler_opt/es/es_worker.py | 40 ------------------------------------ 1 file changed, 40 deletions(-) delete mode 100644 compiler_opt/es/es_worker.py diff --git a/compiler_opt/es/es_worker.py b/compiler_opt/es/es_worker.py deleted file mode 100644 index 22b3ff42..00000000 --- a/compiler_opt/es/es_worker.py +++ /dev/null @@ -1,40 +0,0 @@ -# coding=utf-8 -# Copyright 2020 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Worker for ES Training.""" - -import gin -from typing import List -from compiler_opt.distributed import worker -from compiler_opt.rl import corpus - - -@gin.configurable -class ESWorker(worker.Worker): - """Temporary placeholder worker. - Each time a worker is called, the function value - it will return increases.""" - - def __init__(self, arg, *, kwarg): - self._arg = arg - self._kwarg = kwarg - self.function_value = 0.0 - - def temp_compile(self, policy: bytes, - samples: List[corpus.ModuleSpec]) -> float: - if policy and samples: - self.function_value += 1.0 - return self.function_value - else: - return 0.0 From 2e7bfdbdd3807e0f33f0d77197043463b8e30b1a Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Fri, 13 Sep 2024 20:03:09 +0000 Subject: [PATCH 3/6] Fix invalid function name in comment This patch replaces load_corpus_element with the (presumably) correct function name, load_module_spec, which is defined directly below. This seems to have been an oversight during a refactoring at some point. --- compiler_opt/rl/corpus.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler_opt/rl/corpus.py b/compiler_opt/rl/corpus.py index c351695e..3285611c 100644 --- a/compiler_opt/rl/corpus.py +++ b/compiler_opt/rl/corpus.py @@ -386,7 +386,7 @@ def reset(self): def sample(self, k: int, sort: bool = False) -> List[ModuleSpec]: """Samples `k` module_specs, optionally sorting by size descending. - Use load_corpus_element to get LoadedModuleSpecs - this allows the user + Use load_module_spec to get LoadedModuleSpecs - this allows the user to decide how the loading should happen (e.g. may want to use a threadpool) """ # Note: sampler is intentionally defaulted to a mutable object, as the From 48154f7e5b5a4abe67d6b3e4223c12f3e9e0625e Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Mon, 16 Sep 2024 20:23:07 +0000 Subject: [PATCH 4/6] Add feedback --- compiler_opt/rl/corpus.py | 19 ++++++++++--------- compiler_opt/rl/corpus_test.py | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/compiler_opt/rl/corpus.py b/compiler_opt/rl/corpus.py index 34bdccd3..c406132a 100644 --- a/compiler_opt/rl/corpus.py +++ b/compiler_opt/rl/corpus.py @@ -137,7 +137,7 @@ def reset(self): def __call__(self, k: int, n: int = 20) -> List[ModuleSpec]: """ Args: - k: number of modules to sample + k: The number of corpus elements to sample. n: number of buckets to use """ raise NotImplementedError() @@ -159,7 +159,7 @@ def __call__(self, k: int, n: int = 20) -> List[ModuleSpec]: """ Args: module_specs: list of module_specs to sample from - k: number of modules to sample + k: The number of corpus elements (modules) to sample n: number of buckets to use """ # Credits to yundi@ for the highly optimized algo. @@ -210,7 +210,7 @@ def reset(self): def __call__(self, k: Optional[int], n: int = 10) -> List[ModuleSpec]: """ Args: - k: number of modules to sample + k: number of corpus elements (modules) to sample n: ignored Raises: CorpusExhaustedError if there are fewer than k elements left to sample in @@ -240,8 +240,8 @@ def __call__(self, k: int, n: int = 10) -> List[ModuleSpec]: """Returns the entire corpus as a list of module specs. Args: - k: The number of modules to sample. This must be equal to the number - of modules in the corpus. + k: The number of corpus elements to sample. Given a corpus element is + the whole corpus in this case, k should always be one. n: Ignored for this sampler. Returns: @@ -251,10 +251,11 @@ def __call__(self, k: int, n: int = 10) -> List[ModuleSpec]: ValueError: If the requested number of modules is not equal to the number of modules in the corpus. """ - if len(self._module_specs) != k: + if k != 1: raise ValueError( - f'The number of modules requested {k} is not equal to ' - f'the number of modules in the corpus, {len(self._module_specs)}') + f'The number of corpus elements requested {k} is not equal to ' + f'1, the number of corpus elements (whole corpora) present.' + ) return list(self._module_specs) @@ -418,7 +419,7 @@ def reset(self): self._sampler.reset() def sample(self, k: int, sort: bool = False) -> List[ModuleSpec]: - """Samples `k` module_specs, optionally sorting by size descending. + """Samples `k` corpus elements, optionally sorting by size descending. Use load_corpus_element to get LoadedModuleSpecs - this allows the user decide how the loading should happen (e.g. may want to use a threadpool) diff --git a/compiler_opt/rl/corpus_test.py b/compiler_opt/rl/corpus_test.py index fc7f41f0..c62e2c03 100644 --- a/compiler_opt/rl/corpus_test.py +++ b/compiler_opt/rl/corpus_test.py @@ -277,7 +277,7 @@ def test_whole_corpus_sampler(self): corpus.ModuleSpec(name='large', size=100) ], sampler_type=corpus.WholeCorpusSampler) - sample = cps.sample(4, sort=True) + sample = cps.sample(1, sort=True) self.assertLen(sample, 4) self.assertEqual(sample[0].name, 'large') self.assertEqual(sample[1].name, 'middle') @@ -293,7 +293,7 @@ def test_whole_corpus_sampler_invalid_count(self): ], sampler_type=corpus.WholeCorpusSampler) with self.assertRaises(ValueError): - cps.sample(1) + cps.sample(2) def test_filter(self): cps = corpus.create_corpus_for_testing( From 32ad407ea8f09af00624fc09fa9812d558e38c2a Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Mon, 16 Sep 2024 20:25:23 +0000 Subject: [PATCH 5/6] Reenable pytype attribute errors in env Pytype complains that attribute errors are not reenabled after a temporary disable. This patch fixes that be reenabling them so that it stops throwing the warning while also potentially improving coverage of the code if more is added in the future. --- compiler_opt/rl/env.py | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler_opt/rl/env.py b/compiler_opt/rl/env.py index 1bd96afb..904fd388 100644 --- a/compiler_opt/rl/env.py +++ b/compiler_opt/rl/env.py @@ -359,6 +359,7 @@ def reset(self, module: corpus.LoadedModuleSpec): self._clang_generator.send(None) # pytype: disable=attribute-error self._iclang, self._clang = self._clang_generator.send(module) + # pytype: enable=attribute-error return self._get_observation() def step(self, action: np.ndarray): From 5c89f410ff2bebe934834db401f23d175bbe8034 Mon Sep 17 00:00:00 2001 From: Aiden Grossman Date: Mon, 16 Sep 2024 20:33:56 +0000 Subject: [PATCH 6/6] formatting --- compiler_opt/rl/corpus.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler_opt/rl/corpus.py b/compiler_opt/rl/corpus.py index c406132a..5ca8f4c3 100644 --- a/compiler_opt/rl/corpus.py +++ b/compiler_opt/rl/corpus.py @@ -254,8 +254,7 @@ def __call__(self, k: int, n: int = 10) -> List[ModuleSpec]: if k != 1: raise ValueError( f'The number of corpus elements requested {k} is not equal to ' - f'1, the number of corpus elements (whole corpora) present.' - ) + f'1, the number of corpus elements (whole corpora) present.') return list(self._module_specs)