Change CompilerEnv.step to accept a single action

[sogartar]

No description provided.

[codecov-commenter]

Codecov Report

Merging #606 (78294bc) into development (f8a5117) will decrease coverage by 0.51%.
The diff coverage is 92.68%.

@@               Coverage Diff               @@
##           development     #606      +/-   ##
===============================================
- Coverage        88.25%   87.73%   -0.52%     
===============================================
  Files              114      114              
  Lines             6708     6718      +10     
===============================================
- Hits              5920     5894      -26     
- Misses             788      824      +36     

Impacted Files	Coverage Δ
compiler_gym/random_replay.py	0.00% <0.00%> (ø)
compiler_gym/wrappers/commandline.py	88.46% <86.66%> (-1.93%)	⬇️
compiler_gym/bin/service.py	77.04% <100.00%> (+0.77%)	⬆️
compiler_gym/envs/compiler_env.py	90.70% <100.00%> (-1.13%)	⬇️
compiler_gym/util/gym_type_hints.py	100.00% <100.00%> (ø)
compiler_gym/util/minimize_trajectory.py	92.74% <100.00%> (+0.11%)	⬆️
compiler_gym/wrappers/core.py	93.84% <100.00%> (ø)
compiler_gym/wrappers/time_limit.py	96.42% <100.00%> (+0.13%)	⬆️
compiler_gym/util/logging.py	0.00% <0.00%> (-100.00%)	⬇️
compiler_gym/util/flags/seed.py	0.00% <0.00%> (-100.00%)	⬇️
... and 12 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 f8a5117...78294bc. Read the comment docs.

[ChrisCummins]

Hi @sogartar,

I agree that CompilerEnv.step should accept a single action, but there's a couple of things we need to do before we make this change:

1. We need to add a deprecation warning on env.step([1, 2, 3]), and cut a release of CompilerGym with this warning so that users get advance notice that their code will break.
2. We need to add a new way for specifying a list of actions to run in a single step. The benefit of env.step(actions) over for a in actions: env.step(a) is that the former can be executed in a single RPC invocation, significantly reducing the overhead of round trips to the backend, and removing the need to calculate observation/rewards for each individual step. We measured speedups of ~3x on typical LLVM workloads using this (more details here).

I have a draft PR for (2) already work-in-progress. If you're happy for me to, I can pull in some of your other fixes from this commit and send you the PR when ready.

I made a tracking issue for this, #610.

Cheers,
Chris

[sogartar]

I will add the multistep to this

I have a draft PR for (2) already work-in-progress. If you're happy for me to, I can pull in some of your other fixes from this commit and send you the PR when ready.

That is OK. In this PR I change a lot of tests to not use mltistep actions. These pieces would have to be reverted.

[ChrisCummins]

Thanks! I cherry-picked your commit into #606 and selectively reverted some of the test refactors. It's still WIP, need to figure out the best way for wrappers to interact with multiple different step() implementations.

Cheers,
Chris

[ChrisCummins]

Superseded by #627.