Fix clang-tidy public member naming (#100)

* create benchmark folder * fix clang-tidy public member naming * fix doc
sail-sg · May 2, 2022 · 245510e · 245510e
1 parent 7f02033
commit 245510e
Show file tree

Hide file tree

Showing 43 changed files with 400 additions and 386 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -33,6 +33,7 @@ CheckOptions:
   - { key: readability-identifier-naming.GlobalConstantCase,    value: UPPER_CASE }
   - { key: readability-identifier-naming.MemberCase,            value: lower_case }
   - { key: readability-identifier-naming.MemberSuffix,          value: _          }
+  - { key: readability-identifier-naming.PublicMemberCase,      value: lower_case }
   - { key: readability-identifier-naming.TypeAliasCase,         value: CamelCase  }
   - { key: readability-identifier-naming.ConstantMemberCase,    value: CamelCase  }
   - { key: readability-identifier-naming.ConstantMemberPrefix,  value: k          }

diff --git a/.gitignore b/.gitignore
@@ -146,3 +146,4 @@ pyflakes*
 *.swp
 train_dir
 log
+_vizdoom*
diff --git a/examples/benchmark.py → benchmark/benchmark.py b/examples/benchmark.py → benchmark/benchmark.py
@@ -58,7 +58,7 @@
   parser.add_argument("--total-iter", type=int, default=50000)
   args = parser.parse_args()
   env = envpool.make_gym(
-    "Pong-v5",
+    args.task,
     num_envs=args.num_envs,
     batch_size=args.batch_size,
     num_threads=args.num_threads,

diff --git a/examples/numa_test.sh → benchmark/numa_test.sh b/examples/numa_test.sh → benchmark/numa_test.sh
diff --git a/docs/pages/build.rst b/docs/pages/build.rst
@@ -60,6 +60,8 @@ or `golang <https://golang.org/doc/install>`_ with version >= 1.16:
         # check if successfully installed
         bazel
 
+    See `Issue #87 <https://github.com/sail-sg/envpool/issues/87>`_.
+
 
 Install Other Dependencies
 --------------------------
@@ -113,6 +115,8 @@ This creates a wheel under ``bazel-bin/setup.runfiles/envpool/dist``.
         export HTTPS_PROXY=http://...
         # then run the command to build
 
+    See `Issue #87 <https://github.com/sail-sg/envpool/issues/87>`_.
+
 
 Use Shortcut
 ------------
@@ -154,3 +158,5 @@ system via ``/host``.
     .. code-block:: bash
 
         make docker-dev-cn
+
+    See `Issue #87 <https://github.com/sail-sg/envpool/issues/87>`_.
diff --git a/docs/pages/env.rst b/docs/pages/env.rst
@@ -254,7 +254,7 @@ class for convenience, which follows the definition of ``CartPoleEnvSpec``.
 The following functions are required to override:
 
 - constructor, in this case it is ``CartPoleEnv(const Spec& spec, int env_id)``;
-  you can use ``spec.config_["max_episode_steps"_]`` to extract the value from
+  you can use ``spec.config["max_episode_steps"_]`` to extract the value from
   config;
 - ``bool IsDone()``: return a boolean that indicate whether the current episode
   is finished or not;

diff --git a/envpool/atari/atari_env.h b/envpool/atari/atari_env.h
@@ -103,29 +103,27 @@ class AtariEnv : public Env<AtariEnvSpec> {
   AtariEnv(const Spec& spec, int env_id)
       : Env<AtariEnvSpec>(spec, env_id),
         env_(new ale::ALEInterface()),
-        max_episode_steps_(spec.config_["max_episode_steps"_]),
+        max_episode_steps_(spec.config["max_episode_steps"_]),
         elapsed_step_(max_episode_steps_ + 1),
-        stack_num_(spec.config_["stack_num"_]),
-        frame_skip_(spec.config_["frame_skip"_]),
+        stack_num_(spec.config["stack_num"_]),
+        frame_skip_(spec.config["frame_skip"_]),
         fire_reset_(false),
-        reward_clip_(spec.config_["reward_clip"_]),
-        zero_discount_on_life_loss_(
-            spec.config_["zero_discount_on_life_loss"_]),
-        gray_scale_(spec.config_["gray_scale"_]),
-        episodic_life_(spec.config_["episodic_life"_]),
-        use_inter_area_resize_(spec.config_["use_inter_area_resize"_]),
+        reward_clip_(spec.config["reward_clip"_]),
+        zero_discount_on_life_loss_(spec.config["zero_discount_on_life_loss"_]),
+        gray_scale_(spec.config["gray_scale"_]),
+        episodic_life_(spec.config["episodic_life"_]),
+        use_inter_area_resize_(spec.config["use_inter_area_resize"_]),
         done_(true),
         raw_spec_({kRawHeight, kRawWidth, gray_scale_ ? 1 : 3}),
-        resize_spec_({spec.config_["img_height"_], spec.config_["img_width"_],
+        resize_spec_({spec.config["img_height"_], spec.config["img_width"_],
                       gray_scale_ ? 1 : 3}),
-        transpose_spec_({gray_scale_ ? 1 : 3, spec.config_["img_height"_],
-                         spec.config_["img_width"_]}),
+        transpose_spec_({gray_scale_ ? 1 : 3, spec.config["img_height"_],
+                         spec.config["img_width"_]}),
         resize_img_(resize_spec_),
-        dist_noop_(0, spec.config_["noop_max"_] - 1),
-        rom_path_(
-            GetRomPath(spec.config_["base_path"_], spec.config_["task"_])) {
+        dist_noop_(0, spec.config["noop_max"_] - 1),
+        rom_path_(GetRomPath(spec.config["base_path"_], spec.config["task"_])) {
     env_->setFloat("repeat_action_probability",
-                   spec.config_["repeat_action_probability"_]);
+                   spec.config["repeat_action_probability"_]);
     env_->setInt("random_seed", seed_);
     env_->loadROM(rom_path_);
     action_set_ = env_->getMinimalActionSet();
@@ -258,7 +256,7 @@ class AtariEnv : public Env<AtariEnvSpec> {
     auto* ptr = static_cast<uint8_t*>(maxpool_buf_[0].Data());
     if (maxpool) {
       auto* ptr1 = static_cast<uint8_t*>(maxpool_buf_[1].Data());
-      for (std::size_t i = 0; i < maxpool_buf_[0].size_; ++i) {
+      for (std::size_t i = 0; i < maxpool_buf_[0].size; ++i) {
         ptr[i] = std::max(ptr[i], ptr1[i]);
       }
     }
@@ -282,7 +280,7 @@ class AtariEnv : public Env<AtariEnvSpec> {
         }
       }
     }
-    std::size_t size = tgt.size_;
+    std::size_t size = tgt.size;
     stack_buf_.push_back(std::move(tgt));
     if (push_all) {
       for (auto& s : stack_buf_) {

diff --git a/envpool/classic_control/acrobot.h b/envpool/classic_control/acrobot.h
@@ -49,16 +49,15 @@ using AcrobotEnvSpec = EnvSpec<AcrobotEnvFns>;
 
 class AcrobotEnv : public Env<AcrobotEnvSpec> {
   struct V5 {
-    double s0_{0}, s1_{0}, s2_{0}, s3_{0}, s4_{0};
+    double s0{0}, s1{0}, s2{0}, s3{0}, s4{0};
     V5() = default;
     V5(double s0, double s1, double s2, double s3, double s4)
-        : s0_(s0), s1_(s1), s2_(s2), s3_(s3), s4_(s4) {}
+        : s0(s0), s1(s1), s2(s2), s3(s3), s4(s4) {}
     V5 operator+(const V5& v) const {
-      return V5(s0_ + v.s0_, s1_ + v.s1_, s2_ + v.s2_, s3_ + v.s3_,
-                s4_ + v.s4_);
+      return V5(s0 + v.s0, s1 + v.s1, s2 + v.s2, s3 + v.s3, s4 + v.s4);
     }
     V5 operator*(double v) const {
-      return V5(s0_ * v, s1_ * v, s2_ * v, s3_ * v, s4_ * v);
+      return V5(s0 * v, s1 * v, s2 * v, s3 * v, s4 * v);
     }
   };
 
@@ -82,19 +81,19 @@ class AcrobotEnv : public Env<AcrobotEnvSpec> {
  public:
   AcrobotEnv(const Spec& spec, int env_id)
       : Env<AcrobotEnvSpec>(spec, env_id),
-        max_episode_steps_(spec.config_["max_episode_steps"_]),
+        max_episode_steps_(spec.config["max_episode_steps"_]),
         elapsed_step_(max_episode_steps_ + 1),
         dist_(-kInitRange, kInitRange),
         done_(true) {}
 
   bool IsDone() override { return done_; }
 
   void Reset() override {
-    s_.s0_ = dist_(gen_);
-    s_.s1_ = dist_(gen_);
-    s_.s2_ = dist_(gen_);
-    s_.s3_ = dist_(gen_);
-    s_.s4_ = 0;
+    s_.s0 = dist_(gen_);
+    s_.s1 = dist_(gen_);
+    s_.s2 = dist_(gen_);
+    s_.s3 = dist_(gen_);
+    s_.s4 = 0;
     done_ = false;
     elapsed_step_ = 0;
     WriteState(0.0);
@@ -105,33 +104,33 @@ class AcrobotEnv : public Env<AcrobotEnvSpec> {
     int act = action["action"_];
     float reward = -1.0;
 
-    s_.s4_ = act - 1;
+    s_.s4 = act - 1;
     s_ = Rk4(s_);
-    while (s_.s0_ < -kPi) {
-      s_.s0_ += kPi * 2;
+    while (s_.s0 < -kPi) {
+      s_.s0 += kPi * 2;
     }
-    while (s_.s1_ < -kPi) {
-      s_.s1_ += kPi * 2;
+    while (s_.s1 < -kPi) {
+      s_.s1 += kPi * 2;
     }
-    while (s_.s0_ >= kPi) {
-      s_.s0_ -= kPi * 2;
+    while (s_.s0 >= kPi) {
+      s_.s0 -= kPi * 2;
     }
-    while (s_.s1_ >= kPi) {
-      s_.s1_ -= kPi * 2;
+    while (s_.s1 >= kPi) {
+      s_.s1 -= kPi * 2;
     }
-    if (s_.s2_ < -kMaxVel1) {
-      s_.s2_ = -kMaxVel1;
+    if (s_.s2 < -kMaxVel1) {
+      s_.s2 = -kMaxVel1;
     }
-    if (s_.s3_ < -kMaxVel2) {
-      s_.s3_ = -kMaxVel2;
+    if (s_.s3 < -kMaxVel2) {
+      s_.s3 = -kMaxVel2;
     }
-    if (s_.s2_ > kMaxVel1) {
-      s_.s2_ = kMaxVel1;
+    if (s_.s2 > kMaxVel1) {
+      s_.s2 = kMaxVel1;
     }
-    if (s_.s3_ > kMaxVel2) {
-      s_.s3_ = kMaxVel2;
+    if (s_.s3 > kMaxVel2) {
+      s_.s3 = kMaxVel2;
     }
-    if (-std::cos(s_.s0_) - std::cos(s_.s0_ + s_.s1_) > 1) {
+    if (-std::cos(s_.s0) - std::cos(s_.s0 + s_.s1) > 1) {
       done_ = true;
       reward = 0.0;
     }
@@ -149,11 +148,11 @@ class AcrobotEnv : public Env<AcrobotEnvSpec> {
   }
 
   [[nodiscard]] V5 Derivs(V5 s, double t) const {
-    double theta1 = s.s0_;
-    double theta2 = s.s1_;
-    double dtheta1 = s.s2_;
-    double dtheta2 = s.s3_;
-    double a = s.s4_;
+    double theta1 = s.s0;
+    double theta2 = s.s1;
+    double dtheta1 = s.s2;
+    double dtheta2 = s.s3;
+    double a = s.s4;
     double d1 = kM * kLC * kLC +
                 kM * (kL * kL + kLC * kLC + 2 * kL * kLC * std::cos(theta2)) +
                 kI * 2;
@@ -172,14 +171,14 @@ class AcrobotEnv : public Env<AcrobotEnvSpec> {
 
   void WriteState(float reward) {
     State state = Allocate();
-    state["obs"_][0] = static_cast<float>(std::cos(s_.s0_));
-    state["obs"_][1] = static_cast<float>(std::sin(s_.s0_));
-    state["obs"_][2] = static_cast<float>(std::cos(s_.s1_));
-    state["obs"_][3] = static_cast<float>(std::sin(s_.s1_));
-    state["obs"_][4] = static_cast<float>(s_.s2_);
-    state["obs"_][5] = static_cast<float>(s_.s3_);
-    state["info:state"_][0] = static_cast<float>(s_.s0_);
-    state["info:state"_][1] = static_cast<float>(s_.s1_);
+    state["obs"_][0] = static_cast<float>(std::cos(s_.s0));
+    state["obs"_][1] = static_cast<float>(std::sin(s_.s0));
+    state["obs"_][2] = static_cast<float>(std::cos(s_.s1));
+    state["obs"_][3] = static_cast<float>(std::sin(s_.s1));
+    state["obs"_][4] = static_cast<float>(s_.s2);
+    state["obs"_][5] = static_cast<float>(s_.s3);
+    state["info:state"_][0] = static_cast<float>(s_.s0);
+    state["info:state"_][1] = static_cast<float>(s_.s1);
     state["reward"_] = reward;
   }
 };

diff --git a/envpool/classic_control/cartpole.h b/envpool/classic_control/cartpole.h
@@ -70,7 +70,7 @@ class CartPoleEnv : public Env<CartPoleEnvSpec> {
  public:
   CartPoleEnv(const Spec& spec, int env_id)
       : Env<CartPoleEnvSpec>(spec, env_id),
-        max_episode_steps_(spec.config_["max_episode_steps"_]),
+        max_episode_steps_(spec.config["max_episode_steps"_]),
         elapsed_step_(max_episode_steps_ + 1),
         dist_(-kInitRange, kInitRange),
         done_(true) {}

diff --git a/envpool/classic_control/mountain_car.h b/envpool/classic_control/mountain_car.h
@@ -62,7 +62,7 @@ class MountainCarEnv : public Env<MountainCarEnvSpec> {
  public:
   MountainCarEnv(const Spec& spec, int env_id)
       : Env<MountainCarEnvSpec>(spec, env_id),
-        max_episode_steps_(spec.config_["max_episode_steps"_]),
+        max_episode_steps_(spec.config["max_episode_steps"_]),
         elapsed_step_(max_episode_steps_ + 1),
         dist_(-0.6, -0.4),
         done_(true) {}

diff --git a/envpool/classic_control/mountain_car_continuous.h b/envpool/classic_control/mountain_car_continuous.h
@@ -62,7 +62,7 @@ class MountainCarContinuousEnv : public Env<MountainCarContinuousEnvSpec> {
  public:
   MountainCarContinuousEnv(const Spec& spec, int env_id)
       : Env<MountainCarContinuousEnvSpec>(spec, env_id),
-        max_episode_steps_(spec.config_["max_episode_steps"_]),
+        max_episode_steps_(spec.config["max_episode_steps"_]),
         elapsed_step_(max_episode_steps_ + 1),
         dist_(-0.6, -0.4),
         done_(true) {}

diff --git a/envpool/classic_control/pendulum.h b/envpool/classic_control/pendulum.h
@@ -60,7 +60,7 @@ class PendulumEnv : public Env<PendulumEnvSpec> {
  public:
   PendulumEnv(const Spec& spec, int env_id)
       : Env<PendulumEnvSpec>(spec, env_id),
-        max_episode_steps_(spec.config_["max_episode_steps"_]),
+        max_episode_steps_(spec.config["max_episode_steps"_]),
         elapsed_step_(max_episode_steps_ + 1),
         dist_(-kPi, kPi),
         dist_dot_(-1, 1),

diff --git a/envpool/core/action_buffer_queue.h b/envpool/core/action_buffer_queue.h
@@ -35,9 +35,9 @@
 class ActionBufferQueue {
  public:
   struct ActionSlice {
-    int env_id_;
-    int order_;
-    bool force_reset_;
+    int env_id;
+    int order;
+    bool force_reset;
   };
 
  protected:

diff --git a/envpool/core/action_buffer_queue_test.cc b/envpool/core/action_buffer_queue_test.cc
@@ -37,7 +37,7 @@ TEST(ActionBufferQueueTest, Concurrent) {
   // enqueue all envs
   for (std::size_t i = 0; i < num_envs; ++i) {
     actions.push_back(ActionSlice{
-        .env_id_ = static_cast<int>(i), .order_ = -1, .force_reset_ = false});
+        .env_id = static_cast<int>(i), .order = -1, .force_reset = false});
   }
   queue.EnqueueBulk(actions);
   std::vector<std::atomic<std::size_t>> flag(mul);
@@ -53,9 +53,8 @@ TEST(ActionBufferQueueTest, Concurrent) {
       }
       actions.clear();
       for (std::size_t i = 0; i < env_num[m]; ++i) {
-        actions.push_back(ActionSlice{.env_id_ = static_cast<int>(i),
-                                      .order_ = -1,
-                                      .force_reset_ = false});
+        actions.push_back(ActionSlice{
+            .env_id = static_cast<int>(i), .order = -1, .force_reset = false});
       }
       queue.EnqueueBulk(actions);
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -146,3 +146,4 @@ pyflakes* @@
     *.swp
     train_dir
     log
+    _vizdoom*