Skip to content

Commit acd1011

Browse files
peterkchungpeterkchung
committed
docs: add Python docstrings, fix QUICKSTART output, improve error messages
1 parent 684b527 commit acd1011

9 files changed

Lines changed: 106 additions & 62 deletions

File tree

‎DESIGN.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -852,8 +852,8 @@ Downstream tools can parse these catalogs to:
852852

853853
**Kinematic trees.** *(Resolved)* The `Asset` struct now contains an optional `std::unique_ptr<KinematicTree> kinematic_tree` field. `KinematicTree` holds links, joints, actuators, and sensors with name-based indexing. Option (b) was chosen for type safety. URDF and MJCF importers parse articulated files directly into this representation, and the `ArticulationStage` merges data from source files, sidecar metadata, and YAML config.
854854

855-
**Material library.** The physics stage uses a single `default_material` from config. Real pipelines need a material lookup table (wood, steel, plastic, rubber, ceramic) that maps to density + friction values. Should this be a separate YAML file, embedded in the main config, or an adapter that reads from a database?
855+
**Material library.** *(Resolved)* The `PhysicsStage` supports a `lookup` mode backed by `MaterialLibrary::from_file()`, which loads a YAML material database (`data/materials.yaml`). Materials map names to density, friction, and restitution values. The library is a separate YAML file installed to `${CMAKE_INSTALL_DATADIR}/simforge`.
856856

857-
**Incremental processing.** Currently the pipeline processes all discovered assets every run. Should we support only processing assets newer than their catalog entry? This is essentially `make`-style dependency tracking. Important for large asset libraries but adds complexity.
857+
**Incremental processing.** *(Resolved)* The pipeline computes SHA-256 content hashes (source file + stage config) and writes them to `.catalog.json` files. On subsequent runs, `should_skip_asset()` compares hashes and skips unchanged assets unless `--force` is set. This provides `make`-style dependency tracking with minimal overhead.
858858

859859
**Plugin loading.** Currently adapters are compiled in via CMake flags. Should we support dynamic plugin loading (`dlopen` / shared libraries) so users can add adapters without recompiling simforge? This enables a richer ecosystem but complicates distribution.

‎QUICKSTART.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,12 @@ Use `--dry-run` to see what would be processed without actually running anything
7171
[info] Output: ./sim_ready/
7272
[info] Targets: USD
7373
[info] Stages:
74-
[info] ingest → collision → physics → optimize → validate → export
74+
[info] [OK] ingest
75+
[info] [OK] collision
76+
[info] [OK] physics
77+
[info] [OK] optimize
78+
[info] [OK] validate
79+
[info] [OK] export
7580
[info] Discovered 3 assets in ./raw_assets/
7681
[info] Would process 3 assets
7782
[info] gripper (STEP)

‎python/bind_adapters.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,19 @@ void bind_adapters(py::module_& m) {
1313

1414
m.def("list_importers", []() {
1515
return AdapterManager::instance().list_importers();
16-
});
16+
}, "List names of all registered mesh importers.");
1717
m.def("list_exporters", []() {
1818
return AdapterManager::instance().list_exporters();
19-
});
19+
}, "List names of all registered mesh exporters.");
2020

2121
m.def("available_stages", []() {
2222
return StageRegistry::instance().available();
23-
});
23+
}, "List names of all registered pipeline stages.");
2424
m.def("has_stage", [](const std::string& name) {
2525
return StageRegistry::instance().has(name);
26-
}, py::arg("name"));
26+
}, py::arg("name"), "Check if a pipeline stage is registered.");
2727

2828
m.def("available_validators", []() {
2929
return ValidatorRegistry::instance().available();
30-
});
30+
}, "List names of all registered validators.");
3131
}

‎python/bind_articulation.cpp‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ void bind_articulation(py::module_& m) {
7474

7575
// ── Joint ───────────────────────────────────────────────────────
7676

77-
py::class_<Joint>(m, "Joint")
77+
py::class_<Joint>(m, "Joint", "Kinematic joint connecting two links.")
7878
.def(py::init<>())
7979
.def_readwrite("name", &Joint::name)
8080
.def_readwrite("type", &Joint::type)
@@ -91,7 +91,7 @@ void bind_articulation(py::module_& m) {
9191

9292
// ── Actuator ────────────────────────────────────────────────────
9393

94-
py::class_<Actuator>(m, "Actuator")
94+
py::class_<Actuator>(m, "Actuator", "Motor or controller attached to a joint.")
9595
.def(py::init<>())
9696
.def_readwrite("name", &Actuator::name)
9797
.def_readwrite("joint", &Actuator::joint)
@@ -105,7 +105,7 @@ void bind_articulation(py::module_& m) {
105105

106106
// ── Sensor ──────────────────────────────────────────────────────
107107

108-
py::class_<Sensor>(m, "Sensor")
108+
py::class_<Sensor>(m, "Sensor", "Sensor attached to a link (e.g. IMU, force/torque).")
109109
.def(py::init<>())
110110
.def_readwrite("name", &Sensor::name)
111111
.def_readwrite("type", &Sensor::type)
@@ -118,7 +118,7 @@ void bind_articulation(py::module_& m) {
118118

119119
// ── Link ────────────────────────────────────────────────────────
120120

121-
py::class_<Link>(m, "Link")
121+
py::class_<Link>(m, "Link", "Rigid body in a kinematic tree with visual/collision meshes.")
122122
.def(py::init<>())
123123
.def_readwrite("name", &Link::name)
124124
.def_readwrite("visual_meshes", &Link::visual_meshes)
@@ -133,22 +133,29 @@ void bind_articulation(py::module_& m) {
133133

134134
// ── KinematicTree ───────────────────────────────────────────────
135135

136-
py::class_<KinematicTree>(m, "KinematicTree")
136+
py::class_<KinematicTree>(m, "KinematicTree",
137+
"Articulated structure with links, joints, actuators, and sensors.")
137138
.def(py::init<>())
138139
.def_readwrite("root_link", &KinematicTree::root_link)
139140
.def_readwrite("links", &KinematicTree::links)
140141
.def_readwrite("joints", &KinematicTree::joints)
141142
.def_readwrite("actuators", &KinematicTree::actuators)
142143
.def_readwrite("sensors", &KinematicTree::sensors)
143144
.def("find_link", &KinematicTree::find_link,
144-
py::arg("name"), py::return_value_policy::reference_internal)
145+
py::arg("name"), py::return_value_policy::reference_internal,
146+
"Find a link by name, or None if not found.")
145147
.def("find_joint", &KinematicTree::find_joint,
146-
py::arg("name"), py::return_value_policy::reference_internal)
148+
py::arg("name"), py::return_value_policy::reference_internal,
149+
"Find a joint by name, or None if not found.")
147150
.def("find_actuator_for_joint", &KinematicTree::find_actuator_for_joint,
148-
py::arg("joint_name"), py::return_value_policy::reference_internal)
149-
.def("dof", &KinematicTree::dof)
150-
.def("is_tree", &KinematicTree::is_tree)
151-
.def("build_index", &KinematicTree::build_index)
151+
py::arg("joint_name"), py::return_value_policy::reference_internal,
152+
"Find the actuator attached to a joint, or None.")
153+
.def("dof", &KinematicTree::dof,
154+
"Degrees of freedom (non-fixed joints).")
155+
.def("is_tree", &KinematicTree::is_tree,
156+
"True if the structure forms a valid tree (no cycles).")
157+
.def("build_index", &KinematicTree::build_index,
158+
"Rebuild the internal name-to-index lookup tables.")
152159
.def("__repr__", [](const KinematicTree& kt) {
153160
return "<KinematicTree root='" + kt.root_link + "' links="
154161
+ std::to_string(kt.links.size()) + " joints="

‎python/bind_physics.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ void bind_physics(py::module_& m) {
1616

1717
// ── PhysicsMaterial ─────────────────────────────────────────────
1818

19-
py::class_<PhysicsMaterial>(m, "PhysicsMaterial")
19+
py::class_<PhysicsMaterial>(m, "PhysicsMaterial", "Surface material with density and friction.")
2020
.def(py::init<>())
2121
.def_readwrite("name", &PhysicsMaterial::name)
2222
.def_readwrite("density", &PhysicsMaterial::density)
@@ -30,7 +30,8 @@ void bind_physics(py::module_& m) {
3030

3131
// ── PhysicsProperties ───────────────────────────────────────────
3232

33-
py::class_<PhysicsProperties>(m, "PhysicsProperties")
33+
py::class_<PhysicsProperties>(m, "PhysicsProperties",
34+
"Mass, inertia, and material properties for physics simulation.")
3435
.def(py::init<>())
3536
.def_readwrite("mass", &PhysicsProperties::mass)
3637
.def_readwrite("center_of_mass", &PhysicsProperties::center_of_mass)
@@ -39,15 +40,16 @@ void bind_physics(py::module_& m) {
3940
.def_readwrite("is_static", &PhysicsProperties::is_static)
4041
.def_readwrite("mass_estimated", &PhysicsProperties::mass_estimated)
4142
.def_static("estimate_from_mesh", &PhysicsProperties::estimate_from_mesh,
42-
py::arg("mesh"), py::arg("material"))
43+
py::arg("mesh"), py::arg("material"),
44+
"Estimate mass and inertia from mesh volume and material density.")
4345
.def("__repr__", [](const PhysicsProperties& pp) {
4446
return "<PhysicsProperties mass=" + std::to_string(pp.mass)
4547
+ (pp.is_static ? " static" : "") + ">";
4648
});
4749

4850
// ── CollisionMesh ───────────────────────────────────────────────
4951

50-
py::class_<CollisionMesh>(m, "CollisionMesh")
52+
py::class_<CollisionMesh>(m, "CollisionMesh", "Collision geometry as a set of convex hulls.")
5153
.def(py::init<>())
5254
.def_readwrite("type", &CollisionMesh::type)
5355
.def_readwrite("hulls", &CollisionMesh::hulls)

‎python/bind_pipeline.cpp‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ void bind_pipeline(py::module_& m) {
2727

2828
// ── StageError ──────────────────────────────────────────────────
2929

30-
py::class_<StageError>(m, "StageError")
30+
py::class_<StageError>(m, "StageError", "Error produced by a pipeline stage.")
3131
.def(py::init<>())
3232
.def_readwrite("stage_name", &StageError::stage_name)
3333
.def_readwrite("asset_id", &StageError::asset_id)
@@ -38,7 +38,7 @@ void bind_pipeline(py::module_& m) {
3838

3939
// ── AssetReport ─────────────────────────────────────────────────
4040

41-
py::class_<AssetReport>(m, "AssetReport")
41+
py::class_<AssetReport>(m, "AssetReport", "Per-asset processing report with status, timing, and errors.")
4242
.def(py::init<>())
4343
.def_readwrite("asset_id", &AssetReport::asset_id)
4444
.def_readwrite("asset_name", &AssetReport::asset_name)
@@ -55,15 +55,17 @@ void bind_pipeline(py::module_& m) {
5555

5656
// ── PipelineReport ──────────────────────────────────────────────
5757

58-
py::class_<PipelineReport>(m, "PipelineReport")
58+
py::class_<PipelineReport>(m, "PipelineReport", "Aggregate report for a full pipeline run.")
5959
.def(py::init<>())
6060
.def_readwrite("total_assets", &PipelineReport::total_assets)
6161
.def_readwrite("passed", &PipelineReport::passed)
6262
.def_readwrite("failed", &PipelineReport::failed)
6363
.def_readwrite("total_time_ms", &PipelineReport::total_time_ms)
6464
.def_readwrite("asset_reports", &PipelineReport::asset_reports)
65-
.def("print_summary", &PipelineReport::print_summary)
66-
.def("write_json", &PipelineReport::write_json, py::arg("path"))
65+
.def("print_summary", &PipelineReport::print_summary,
66+
"Print a human-readable summary to the log.")
67+
.def("write_json", &PipelineReport::write_json, py::arg("path"),
68+
"Write the report as JSON to the given file path.")
6769
.def("__repr__", [](const PipelineReport& r) {
6870
return "<PipelineReport assets=" + std::to_string(r.total_assets)
6971
+ " passed=" + std::to_string(r.passed)
@@ -72,20 +74,24 @@ void bind_pipeline(py::module_& m) {
7274

7375
// ── PipelineConfig ──────────────────────────────────────────────
7476

75-
py::class_<PipelineConfig>(m, "PipelineConfig")
77+
py::class_<PipelineConfig>(m, "PipelineConfig",
78+
"Pipeline configuration parsed from YAML. Use from_file() or from_string().")
7679
.def(py::init<>())
7780
.def_readwrite("source_dir", &PipelineConfig::source_dir)
7881
.def_readwrite("output_dir", &PipelineConfig::output_dir)
7982
.def_readwrite("target_formats", &PipelineConfig::target_formats)
8083
.def_readwrite("stage_order", &PipelineConfig::stage_order)
8184
.def_readwrite("threads", &PipelineConfig::threads)
8285
.def_readwrite("force", &PipelineConfig::force)
83-
.def_static("from_file", &PipelineConfig::from_file, py::arg("config_path"))
84-
.def_static("from_string", &PipelineConfig::from_string, py::arg("yaml_str"));
86+
.def_static("from_file", &PipelineConfig::from_file, py::arg("config_path"),
87+
"Load pipeline config from a YAML file.")
88+
.def_static("from_string", &PipelineConfig::from_string, py::arg("yaml_str"),
89+
"Parse pipeline config from a YAML string.");
8590

8691
// ── Asset ───────────────────────────────────────────────────────
8792

88-
py::class_<Asset>(m, "Asset")
93+
py::class_<Asset>(m, "Asset",
94+
"A 3D asset with meshes, physics, collision, and optional articulation.")
8995
.def(py::init<>())
9096
.def_readwrite("id", &Asset::id)
9197
.def_readwrite("name", &Asset::name)
@@ -117,8 +123,10 @@ void bind_pipeline(py::module_& m) {
117123
.def_property("metadata",
118124
[](const Asset& a) { return json_to_py(a.metadata); },
119125
[](Asset& a, const py::object& obj) { a.metadata = py_to_json(obj); })
120-
.def("is_articulated", &Asset::is_articulated)
121-
.def("all_validations_passed", &Asset::all_validations_passed)
126+
.def("is_articulated", &Asset::is_articulated,
127+
"True if this asset has a kinematic tree (joints, links).")
128+
.def("all_validations_passed", &Asset::all_validations_passed,
129+
"True if every validation check passed.")
122130
.def("__repr__", [](const Asset& a) {
123131
return "<Asset '" + a.name + "' meshes="
124132
+ std::to_string(a.meshes.size())
@@ -127,15 +135,22 @@ void bind_pipeline(py::module_& m) {
127135

128136
// ── Pipeline ────────────────────────────────────────────────────
129137

130-
py::class_<Pipeline>(m, "Pipeline")
131-
.def(py::init<PipelineConfig>(), py::arg("config"))
132-
.def("build", &Pipeline::build)
138+
py::class_<Pipeline>(m, "Pipeline",
139+
"Asset processing pipeline. Call build() then run().")
140+
.def(py::init<PipelineConfig>(), py::arg("config"),
141+
"Construct a pipeline from a PipelineConfig.")
142+
.def("build", &Pipeline::build,
143+
"Build the stage chain from config. Must be called before run().")
133144
.def("run", [](Pipeline& p) {
134145
py::gil_scoped_release release;
135146
return p.run();
136-
})
137-
.def("run_single", &Pipeline::run_single, py::arg("asset"))
138-
.def("dry_run", &Pipeline::dry_run)
139-
.def("discover_assets", &Pipeline::discover_assets)
140-
.def("stage_names", &Pipeline::stage_names);
147+
}, "Run the full pipeline on all discovered assets. Releases the GIL.")
148+
.def("run_single", &Pipeline::run_single, py::arg("asset"),
149+
"Process a single asset through all stages.")
150+
.def("dry_run", &Pipeline::dry_run,
151+
"Preview what would be processed without running stages.")
152+
.def("discover_assets", &Pipeline::discover_assets,
153+
"Scan the source directory and return discovered assets.")
154+
.def("stage_names", &Pipeline::stage_names,
155+
"Return the names of all built stages.");
141156
}

‎python/bind_types.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void bind_types(py::module_& m) {
4545

4646
// ── Vec3 ────────────────────────────────────────────────────────
4747

48-
py::class_<Vec3>(m, "Vec3")
48+
py::class_<Vec3>(m, "Vec3", "3D vector with x, y, z components.")
4949
.def(py::init<>())
5050
.def(py::init<float, float, float>(), py::arg("x"), py::arg("y"), py::arg("z"))
5151
.def_readwrite("x", &Vec3::x)
@@ -62,7 +62,7 @@ void bind_types(py::module_& m) {
6262

6363
// ── AABB ────────────────────────────────────────────────────────
6464

65-
py::class_<AABB>(m, "AABB")
65+
py::class_<AABB>(m, "AABB", "Axis-aligned bounding box.")
6666
.def(py::init<>())
6767
.def_readwrite("min", &AABB::min)
6868
.def_readwrite("max", &AABB::max)
@@ -93,20 +93,20 @@ void bind_types(py::module_& m) {
9393

9494
// ── Mesh ────────────────────────────────────────────────────────
9595

96-
py::class_<Mesh>(m, "Mesh")
96+
py::class_<Mesh>(m, "Mesh", "Triangle mesh with vertices, normals, faces, and UVs.")
9797
.def(py::init<>())
9898
.def_readwrite("name", &Mesh::name)
9999
.def_readwrite("vertices", &Mesh::vertices)
100100
.def_readwrite("normals", &Mesh::normals)
101101
.def_readwrite("faces", &Mesh::faces)
102102
.def_readwrite("uvs", &Mesh::uvs)
103103
.def_readwrite("bounds", &Mesh::bounds)
104-
.def("triangle_count", &Mesh::triangle_count)
105-
.def("vertex_count", &Mesh::vertex_count)
106-
.def("empty", &Mesh::empty)
107-
.def("recompute_bounds", &Mesh::recompute_bounds)
108-
.def("compute_volume", &Mesh::compute_volume)
109-
.def("is_watertight", &Mesh::is_watertight)
104+
.def("triangle_count", &Mesh::triangle_count, "Number of triangles.")
105+
.def("vertex_count", &Mesh::vertex_count, "Number of vertices.")
106+
.def("empty", &Mesh::empty, "True if the mesh has no vertices.")
107+
.def("recompute_bounds", &Mesh::recompute_bounds, "Recompute the axis-aligned bounding box.")
108+
.def("compute_volume", &Mesh::compute_volume, "Compute signed volume (accurate for watertight meshes).")
109+
.def("is_watertight", &Mesh::is_watertight, "True if every edge is shared by exactly two triangles.")
110110
.def("__repr__", [](const Mesh& mesh) {
111111
return "<Mesh '" + mesh.name + "' verts=" + std::to_string(mesh.vertex_count())
112112
+ " tris=" + std::to_string(mesh.triangle_count()) + ">";
@@ -138,7 +138,7 @@ void bind_types(py::module_& m) {
138138

139139
// ── ValidationResult ────────────────────────────────────────────
140140

141-
py::class_<ValidationResult>(m, "ValidationResult")
141+
py::class_<ValidationResult>(m, "ValidationResult", "Result of a single validation check.")
142142
.def(py::init<>())
143143
.def_readwrite("passed", &ValidationResult::passed)
144144
.def_readwrite("check_name", &ValidationResult::check_name)

0 commit comments

Comments
 (0)