Skip to content

Commit 0099d73

Browse files
Trinkle23897Trinkle23897
authored
[envpool] finish phase-1 native dependency upgrades (#335)
## Summary - Problem: phase-1 native dependencies were still pinned to older releases, and the repo's current lint/toolchain setup did not cleanly survive the native upgrade work on Linux. - Scope: upgrade the remaining phase-1 native third-party pieces used by the current repo and apply only the mechanical code/build cleanups needed to keep Bazel, lint, and tests green on dev. - Outcome: the native dependency pass now covers NASM, libjpeg-turbo, Boost, and the vendored ThreadPool compatibility edge, with `make lint` and full Bazel tests passing on `dev-0`. This finishes the first native-upgrade pass without pulling in the larger benchmark modernization or Bazel rules migration work. ## Technical Details - Approach: vendor NASM 3.01's Linux config header, migrate the libjpeg-turbo BUILD to the 3.x source layout, patch vendored ThreadPool for `std::invoke_result_t`, pin repo lint to `clang-tidy-18`, and make small semantics-preserving code cleanups where the newer lint stack flagged issues. - Code pointers: - `envpool/workspace0.bzl`: native archive versions, NASM/threadpool wiring, and updated fetch metadata. - `third_party/jpeg/jpeg.BUILD`: libjpeg-turbo 3.x layout, generated headers, and SIMD rule updates. - `third_party/nasm/config.h`: checked-in Linux configure output used to keep NASM 3.01 reproducible under Bazel. - `third_party/threadpool/invoke_result.patch`: C++17 compatibility patch replacing `std::result_of`. - `Makefile`: `clang-tidy-18` install/pin wrapper used by repo lint. - Notes: the first full `make bazel-test` on `dev-0` hit one `//envpool/dummy:dummy_envpool_test` flake; the single-target rerun, `--runs_per_test=3`, and a second full `make bazel-test` all passed. ## Test Plan ### Automated - `brix ssh dev-0 -C -- 'exec bash -il -c "cd /root/code/envpool && make lint"'`: passed - `brix ssh dev-0 -C -- 'exec bash -il -c "cd /root/code/envpool && make bazel-test"'`: passed on rerun (`30/30`) - `brix ssh dev-0 -C -- 'exec bash -il -c "cd /root/code/envpool && USE_BAZEL_VERSION=6.0.0 /root/code/openai/project/dotslash-gen/bin/bazelisk test //envpool/dummy:dummy_envpool_test --config=test --test_output=all --nocache_test_results"'`: passed - `brix ssh dev-0 -C -- 'exec bash -il -c "cd /root/code/envpool && USE_BAZEL_VERSION=6.0.0 /root/code/openai/project/dotslash-gen/bin/bazelisk test //envpool/dummy:dummy_envpool_test --config=test --test_output=errors --nocache_test_results --runs_per_test=3"'`: passed ### Suggested Manual - `git diff origin/main...HEAD`: review the third-party archive/version changes and BUILD-file migrations. - Review `third_party/jpeg/jpeg.BUILD` and `third_party/nasm/config.h`: confirm the checked-in NASM config header and libjpeg 3.x Bazelization strategy are acceptable.
1 parent 4180328 commit 0099d73

34 files changed

Lines changed: 1473 additions & 375 deletions

‎.clang-tidy‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,3 @@ CheckOptions:
4949
- { key: readability-identifier-naming.VariableCase, value: lower_case }
5050
WarningsAsErrors: '*'
5151
HeaderFilterRegex: '/envpool/'
52-
AnalyzeTemporaryDtors: true

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,4 @@ log
148148
_vizdoom*
149149
MUJOCO_LOG.TXT
150150
.vscode/
151+
plan.md

‎Makefile‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ BAZEL = USE_BAZEL_VERSION=$(BAZEL_VERSION) $(BAZELISK_BIN)
1313
DATE = $(shell date "+%Y-%m-%d")
1414
DOCKER_TAG = $(DATE)-$(COMMIT_HASH)
1515
DOCKER_USER = trinkle23897
16-
PATH := $(HOME)/go/bin:$(PATH)
16+
CLANG_TIDY_MAJOR = 18
17+
CLANG_TIDY_BIN = clang-tidy-$(CLANG_TIDY_MAJOR)
18+
CLANG_TIDY_WRAPPER_DIR = $(HOME)/.cache/$(PROJECT_NAME)/bin
19+
PATH := $(CLANG_TIDY_WRAPPER_DIR):$(HOME)/go/bin:$(PATH)
1720

1821
# installation
1922

@@ -38,7 +41,9 @@ clang-format-install:
3841
command -v clang-format || sudo apt-get install -y clang-format
3942

4043
clang-tidy-install:
41-
command -v clang-tidy || sudo apt-get install -y clang-tidy
44+
command -v $(CLANG_TIDY_BIN) || sudo apt-get install -y $(CLANG_TIDY_BIN)
45+
mkdir -p $(CLANG_TIDY_WRAPPER_DIR)
46+
ln -sf $$(command -v $(CLANG_TIDY_BIN)) $(CLANG_TIDY_WRAPPER_DIR)/clang-tidy
4247

4348
go-install:
4449
# requires go >= 1.16

‎envpool/atari/atari_env.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,10 @@ class AtariEnv : public Env<AtariEnvSpec> {
141141
}
142142
// init buf
143143
for (int i = 0; i < 2; ++i) {
144-
maxpool_buf_.emplace_back(Array(raw_spec_));
144+
maxpool_buf_.emplace_back(raw_spec_);
145145
}
146146
for (int i = 0; i < stack_num_; ++i) {
147-
stack_buf_.emplace_back(Array(transpose_spec_));
147+
stack_buf_.emplace_back(transpose_spec_);
148148
}
149149
}
150150

‎envpool/box2d/car_dynamics.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,8 @@ void Car::Step(float dt) {
210210

211211
w->body->ApplyForceToCenter(
212212
{
213-
static_cast<float>(p_force * side.x + f_force * forw.x),
214-
static_cast<float>(p_force * side.y + f_force * forw.y),
213+
p_force * side.x + f_force * forw.x,
214+
p_force * side.y + f_force * forw.y,
215215
},
216216
true);
217217
}
@@ -237,8 +237,8 @@ void Car::Draw(const cv::Mat& surf, float zoom,
237237
poly.clear();
238238
for (const auto& vec_tmp : p->poly) {
239239
auto v = RotateRad(vec_tmp, angle);
240-
poly.emplace_back(cv::Point(v.x * zoom + translation[0],
241-
v.y * zoom + translation[1]));
240+
poly.emplace_back(v.x * zoom + translation[0],
241+
v.y * zoom + translation[1]);
242242
}
243243
cv::polylines(surf, poly, false, p->color, 2);
244244
}
@@ -260,8 +260,8 @@ void Car::Draw(const cv::Mat& surf, float zoom,
260260
for (int j = 0; j < shape->m_count; j++) {
261261
auto vec_tmp = Multiply(trans, shape->m_vertices[j]);
262262
auto v = RotateRad(vec_tmp, angle);
263-
poly.emplace_back(cv::Point(v.x * zoom + translation[0],
264-
v.y * zoom + translation[1]));
263+
poly.emplace_back(v.x * zoom + translation[0],
264+
v.y * zoom + translation[1]);
265265
}
266266
cv::fillPoly(surf, poly, color);
267267

@@ -298,8 +298,8 @@ void Car::Draw(const cv::Mat& surf, float zoom,
298298
for (const auto& vec : white_poly) {
299299
auto vec_tmp = Multiply(trans, vec);
300300
auto v = RotateRad(vec_tmp, angle);
301-
poly.emplace_back(cv::Point(v.x * zoom + translation[0],
302-
v.y * zoom + translation[1]));
301+
poly.emplace_back(v.x * zoom + translation[0],
302+
v.y * zoom + translation[1]);
303303
}
304304
cv::fillPoly(surf, poly, kWheelWhite);
305305
}

‎envpool/box2d/car_dynamics.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <box2d/box2d.h>
2222

2323
#include <cmath>
24+
#include <cstdint>
2425
#include <deque>
2526
#include <memory>
2627
#include <random>
@@ -79,7 +80,7 @@ static const cv::Scalar kWheelColor(0, 0, 0);
7980
static const cv::Scalar kWheelWhite(77, 77, 77);
8081
static const cv::Scalar kMudColor(0, 102, 102);
8182

82-
enum UserDataType { INVALID = 1000, WHEEL_TYPE, TILE_TYPE };
83+
enum UserDataType : std::uint16_t { INVALID = 1000, WHEEL_TYPE, TILE_TYPE };
8384

8485
class Particle {
8586
public:

‎envpool/box2d/car_racing_env.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ bool CarRacingBox2dEnv::CreateTrack(std::mt19937* gen) {
292292
t->idx = i;
293293
t->body->GetFixtureList()[0].SetSensor(true);
294294
roads_.push_back(t);
295-
roads_poly_.emplace_back(std::make_pair(roads_vertices, t->road_color));
295+
roads_poly_.emplace_back(roads_vertices, t->road_color);
296296

297297
if (border[i]) {
298298
auto side = Sign(beta2 - beta1);
@@ -313,7 +313,7 @@ bool CarRacingBox2dEnv::CreateTrack(std::mt19937* gen) {
313313
std::array<b2Vec2, 4> border_vertices = {b1_l, b1_r, b2_r, b2_l};
314314
cv::Scalar border_color =
315315
(i % 2 == 0) ? cv::Scalar(255, 255, 255) : cv::Scalar(0, 0, 255);
316-
roads_poly_.emplace_back(std::make_pair(border_vertices, border_color));
316+
roads_poly_.emplace_back(border_vertices, border_color);
317317
}
318318
}
319319
track_ = current_track;
@@ -435,7 +435,7 @@ void CarRacingBox2dEnv::DrawColoredPolygon(
435435
auto f_roated = RotateRad(f, angle);
436436
f_roated = {f_roated[0] * zoom + translation[0],
437437
f_roated[1] * zoom + translation[1]};
438-
poly.emplace_back(cv::Point(f_roated[0], f_roated[1]));
438+
poly.emplace_back(f_roated[0], f_roated[1]);
439439
if (-kMaxShapeDim <= f_roated[0] &&
440440
f_roated[0] <= static_cast<float>(kWindowW) + kMaxShapeDim &&
441441
-kMaxShapeDim <= f_roated[1] &&

‎envpool/classic_control/pendulum.h‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,25 @@ class PendulumEnv : public Env<PendulumEnvSpec> {
7777
void Step(const Action& action) override {
7878
done_ = (++elapsed_step_ >= max_episode_steps_);
7979
float act = action["action"_];
80-
double u = act < -kMaxTorque ? -kMaxTorque
81-
: act > kMaxTorque ? kMaxTorque
82-
: act;
80+
double u = act;
81+
if (act < -kMaxTorque) {
82+
u = -kMaxTorque;
83+
} else if (act > kMaxTorque) {
84+
u = kMaxTorque;
85+
}
8386
double cost =
8487
theta_ * theta_ + 0.1 * theta_dot_ * theta_dot_ + 0.001 * u * u;
8588
double new_theta_dot =
8689
theta_dot_ + 3 * (kGravity / 2 * std::sin(theta_) + u) * kDt;
8790
if (version_ == 0) {
8891
theta_ += new_theta_dot * kDt;
8992
}
90-
theta_dot_ = new_theta_dot < -kMaxSpeed ? -kMaxSpeed
91-
: new_theta_dot > kMaxSpeed ? kMaxSpeed
92-
: new_theta_dot;
93+
theta_dot_ = new_theta_dot;
94+
if (new_theta_dot < -kMaxSpeed) {
95+
theta_dot_ = -kMaxSpeed;
96+
} else if (new_theta_dot > kMaxSpeed) {
97+
theta_dot_ = kMaxSpeed;
98+
}
9399
if (version_ == 1) {
94100
theta_ += new_theta_dot * kDt;
95101
}

‎envpool/core/array.h‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,15 @@ class Array {
7676
*/
7777
explicit Array(const ShapeSpec& spec)
7878
: Array(spec, nullptr, [](char* /*unused*/) {}) {
79-
ptr_.reset(new char[size * element_size](),
80-
[](const char* p) { delete[] p; });
79+
auto buffer = std::make_shared<std::vector<char>>(size * element_size);
80+
ptr_ = std::shared_ptr<char>(buffer, buffer->data());
8181
}
8282

8383
/**
8484
* Take multidimensional index into the Array.
8585
*/
8686
template <typename... Index>
87-
inline Array operator()(Index... index) const {
87+
Array operator()(Index... index) const {
8888
constexpr std::size_t num_index = sizeof...(Index);
8989
DCHECK_GE(ndim, num_index);
9090
std::size_t offset = 0;
@@ -101,7 +101,7 @@ class Array {
101101
/**
102102
* Index operator of array, takes the index along the first axis.
103103
*/
104-
inline Array operator[](int index) const { return this->operator()(index); }
104+
Array operator[](int index) const { return this->operator()(index); }
105105

106106
/**
107107
* Take a slice at the first axis of the Array.
@@ -166,21 +166,17 @@ class Array {
166166
/**
167167
* Size of axis `dim`.
168168
*/
169-
[[nodiscard]] inline std::size_t Shape(std::size_t dim) const {
170-
return shape_[dim];
171-
}
169+
[[nodiscard]] std::size_t Shape(std::size_t dim) const { return shape_[dim]; }
172170

173171
/**
174172
* Shape
175173
*/
176-
[[nodiscard]] inline const std::vector<std::size_t>& Shape() const {
177-
return shape_;
178-
}
174+
[[nodiscard]] const std::vector<std::size_t>& Shape() const { return shape_; }
179175

180176
/**
181177
* Pointer to the raw memory.
182178
*/
183-
[[nodiscard]] inline void* Data() const { return ptr_.get(); }
179+
[[nodiscard]] void* Data() const { return ptr_.get(); }
184180

185181
/**
186182
* Truncate the Array. Return a new Array that shares the same memory
@@ -225,14 +221,14 @@ class TArray : public Array {
225221
* Take multidimensional index into the Array.
226222
*/
227223
template <typename... Index>
228-
inline TArray operator()(Index... index) const {
224+
TArray operator()(Index... index) const {
229225
return TArray(Array::operator()(index...));
230226
}
231227

232228
/**
233229
* Index operator of array, takes the index along the first axis.
234230
*/
235-
inline TArray operator[](int index) const { return this->operator()(index); }
231+
TArray operator[](int index) const { return this->operator()(index); }
236232

237233
/**
238234
* Take a slice at the first axis of the Array.

‎envpool/core/dict.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ class Value {
4444
template <char... C>
4545
class Key {
4646
public:
47-
static constexpr const inline char kStr[sizeof...(C) + 1]{C..., // NOLINT
48-
'\0'};
49-
static constexpr const inline std::string_view kStrView{kStr, sizeof...(C)};
47+
static constexpr const char kStr[sizeof...(C) + 1]{C..., // NOLINT
48+
'\0'};
49+
static constexpr const std::string_view kStrView{kStr, sizeof...(C)};
5050
template <typename Type>
51-
static constexpr inline auto Bind(Type&& v) {
51+
static constexpr auto Bind(Type&& v) {
5252
return Value<Key, Type>(std::forward<Type>(v));
5353
}
54-
static inline std::string Str() { return {kStrView.data(), kStrView.size()}; }
54+
static std::string Str() { return {kStrView.data(), kStrView.size()}; }
5555
};
5656

5757
template <class CharT, CharT... CS>
@@ -88,7 +88,7 @@ class NamedVector {
8888
NamedVector(const Keys& keys, Vector* values) : values_(values) {}
8989
template <typename Key,
9090
std::enable_if_t<any_match<Key, Keys>::value, bool> = true>
91-
inline decltype(auto) operator[](const Key& key) const {
91+
decltype(auto) operator[](const Key& key) const {
9292
return Take<Key, Keys, Vector&>(key, *values_);
9393
}
9494

@@ -169,12 +169,12 @@ class Dict : public std::decay_t<TupleOrVector> {
169169
*/
170170
template <typename Key,
171171
std::enable_if_t<any_match<Key, Keys>::value, bool> = true>
172-
inline decltype(auto) operator[](const Key& key) {
172+
decltype(auto) operator[](const Key& key) {
173173
return Take<Key, Keys, Values&>(key, *this);
174174
}
175175
template <typename Key,
176176
std::enable_if_t<any_match<Key, Keys>::value, bool> = true>
177-
inline decltype(auto) operator[](const Key& key) const {
177+
decltype(auto) operator[](const Key& key) const {
178178
return Take<Key, Keys, const Values&>(key, *this);
179179
}
180180

0 commit comments

Comments
 (0)