Repository navigation
fix(ci): green the Rust job — 303 test binaries in 9.6 min, up from 38 in 22.7 - #776
Conversation
|
Choji review — Looks good The change is sound: all seven test failures are correctly diagnosed and resolved (four by fixing stale hardcoded segment counts, three by quarantining known #758 regressions), the physics upstream break is properly fixed, and the CI improvements are well-reasoned. One low-severity note on the workspace-wide opt-level 2 change is worth flagging. No findings — looks good. Verified by ChojiChoji ran your change live — checks failed. 3 checks.
Checks marked environment failed in Choji's sandbox, not against your PR. They never block. No live preview was run (this is a pure Rust/CI change with no web UI surface). The two JS checks ( No issues found in the running app. Reviewed |
![chojiai[bot]](https://avatars.githubusercontent.com/in/3490784?s=60&v=4){kind=small}
Dismissing prior approval to re-evaluate b461c8b.
ecto
changed the title
cargo test from 25 min to ~1
Dismissing prior approval to re-evaluate efcdc7b.
`cargo test --workspace` was red on main with 7 failing tests, but only ever reported one at a time: cargo stops at the first failing test BINARY, so CI reached 38 of ~200 binaries and each fix revealed the next failure. A --no-fail-fast sweep found all 7 at once (3678 passing). Four were not kernel bugs — the tests encoded a stale convention. #758 moved boolean-result rims onto the sag-adaptive canonical grid (split.rs::arc_segments), so a through hole discretizes at max(requested, sag) points: 112 for r=2.5, not circle_segments=32. The closed forms still assumed the inscribed 32-gon. Recomputing with the kernel's own rim count reproduces the measured volume to 1.9e-16 against a 1e-12 gate, so arc_segments is now public+documented and the tests ask it instead of pinning a copy of the convention: m2_through_hole_dv_dr_matches_closed_form_and_fd m2_rim_nodes_implicit_diff_matches_fd m10_hole_second_derivative_matches_closed_form_and_fd m2_hole_radius_synthesized Three are real #758 defects, quarantined with measured notes (not fixed): control_static_group_volume — a union of two Z-DISJOINT solids loses volume. Reduced to a 2-solid repro: base ∪ stator1 = 38313.784 vs 42235.415 (-3921.631), base topping out at z=25 and stator1 at z∈[26, 28.7]. All leaves correct to <1.1 mm³; the whole error appears in that one step and stays flat over 6 later unions. test_flanged_hub_mesh_validation — 1 degenerate triangle (expected 0). through_hole_reconstructs_full_circles — STEP exports 6 CIRCLEs, not 8. Speed: tests run in the dev profile, which was unoptimized. The torr catalogue alone was 20.8 min of the job's 25.4 min `cargo test`; at opt-level 2 the same 24 tests take 7.9s (158.4s at opt-level 0 on the same machine) — 20x, with debug_assertions and overflow checks still on, unlike --release. This generalizes the existing vcad-kernel-fea override. m3_flywheel_optimize is marked a perf canary: it PASSES, but 250 L-BFGS iterations x 5 booleans is ~13 min locally and would push the job toward its 75-min cap now that --no-fail-fast makes CI actually reach it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…w takes dt) phyz main added a `dt` parameter to `phyz::contact::assemble` (ecto/phyz#36-38) so the MuJoCo-style solref bias can turn a penetration depth into a separating velocity. vcad's call site still passed 6 arguments, so EVERY Rust CI job — stable, nightly and the WASM build — failed to compile vcad-kernel-physics. This is upstream drift, not a vcad change: CI clones phyz from main unpinned, so the break landed on vcad main without any vcad commit. Passing `dt` alone would double-stabilize: vcad applied its own capped Baumgarte push to `free_velocity` right after assemble, which is the same job phyz's solref now does per contact row (ContactMaterial::default() gives timeconst 0.02 / dampratio 1.0, removing ~9% of penetration per step at dt=1/240, against vcad's 20% of depth-minus-slop). Upstream owns this now, so the hand-rolled bias goes and phyz's runs instead. Verified: 109 vcad-kernel-physics + vcad-sim tests pass, including contact_observation, cross_scale, free_joint, servo_stability and m11_contact_gradient — the tests that would catch a stabilization change. Full workspace as CI runs it: 3926 passed, 0 failed, 61 ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With tests now running optimized, the cold-cache split on CI is 21.5 min compiling vs 9.6 min actually running tests — compilation is the bottleneck. rust-cache excludes workspace members by default, so all ~90 vcad crates rebuilt every run even when a PR touched one. The wasm job already sets this. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
efcdc7b to
a5ed944
Compare
Two stranded fragments sat above the function: an orphaned 'Compute the U parameter for a point on a cylinder surface' line, and a pre-#758 contract saying the count 'must stay at the caller's count — sag-adaptive densification here reopens every analytic/frozen seam'. The second is directly contradicted by the code, which returns max(requested, sag_count). Making the function public put both in rustdoc. Rewritten as one block. The old warning is kept as a caveat rather than deleted: the display tessellator re-samples surviving ANALYTIC circle boundaries at exactly the caller's n, so densifying makes a frozen rim disagree with an analytic neighbour — which is a plausible (unproven) cause of the quarantined through_hole_reconstructs_full_circles, and the first place to look. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Fixed in 7b609f6 — good catch, and it was worse than a leftover fragment. Two stale blocks were stacked above Rewritten as one block. I kept the old warning as a caveat rather than deleting it: the display tessellator re-samples surviving analytic circle boundaries at exactly the caller's 🤖 Addressed by Claude Code |
Dismissing prior approval to re-evaluate 7b609f6.
The problem, and why it kept moving
cargo test --workspacewas red on main. It looked like "two failing tests" because cargo stops at the first failing test binary — CI only ever reached 38 of ~300 binaries, so each fix revealed the next failure instead of turning the job green. (#774 quarantined two; main went red again on a third.)A
--no-fail-fastsweep found the whole set at once: 7 failures, 3678 passing. This PR resolves all 7, fixes an unrelated upstream break, and adds--no-fail-fastso the suite can never hide a queue again.Measured result on CI
torr_boolean_catalogue8x the tests actually executed, in 42% of the time.
Four were stale tests, not kernel bugs — fixed properly
#758 moved boolean-result rims onto the sag-adaptive canonical grid (
split.rs::arc_segments), so a rim discretizes atmax(requested, sag_count)points — 112 for r=2.5, notcircle_segments = 32. Four differentiable-seam tests still hardcoded the inscribed 32-gon.Recomputing with the kernel's actual rim count reproduces the measured volume exactly:
The kernel is also strictly more accurate now (hole area 19.6247 vs the true circle's 19.6350; the old 32-gon gave 19.5090).
arc_segmentsis now public and documented, and the tests ask it rather than pinning their own copy of the convention:m2_through_hole_dv_dr_matches_closed_form_and_fd,m2_rim_nodes_implicit_diff_matches_fd,m10_hole_second_derivative_matches_closed_form_and_fd,m2_hole_radius_synthesized.Three are real #758 defects — quarantined, not fixed
control_static_group_volume— the useful one. Reduced to a two-solid repro:base ∪ stator1= 38313.784 vs 42235.415 exact (−3921.631), where base tops out at z=25 and stator1 spans z∈[26, 28.7] — the two solids do not touch. A union of Z-disjoint solids is losing volume. Every leaf measures correct to <1.1 mm³, and in the full tree the entire error appears at that one step and stays flat across 6 later unions — one bad operand pair, not accumulation.test_flanged_hub_mesh_validation— 1 degenerate triangle (expected 0); not diagnosed past the count.through_hole_reconstructs_full_circles— STEP exports 6CIRCLEentities instead of 8; export fidelity, not a volume error.An unrelated upstream break, also fixed
Partway through, every Rust job (stable, nightly, WASM) started failing to compile: phyz main added a
dtparameter tophyz::contact::assemble(ecto/phyz#36-38) and vcad's call site still passed 6 arguments. CI clones phyz from main unpinned, so this landed on vcad main with no vcad commit.Passing
dtalone would double-stabilize — vcad applied its own capped Baumgarte push right afterassemble, which is exactly what phyz's solref bias now does per contact row (~9%/step at dt=1/240 vs vcad's 20% of depth-minus-slop). Upstream owns this now, so the hand-rolled bias is removed. Verified by the 109 physics tests includingcontact_observation,servo_stabilityandm11_contact_gradient.Worth a follow-up: an unpinned sibling clone means any phyz merge can redden vcad CI.
Speed: what changed and what it cost
Tests ran in the dev profile at opt-level 0 — brutal for a numerics kernel. Same 24 tests, same machine: 158.4 s → 7.9 s at opt-level 2, and on CI the catalogue went 20.8 min → 0.89 min. Unlike
--releasethis keepsdebug_assertionsand overflow checks. It generalizes the[profile.dev.package.vcad-kernel-fea]override already in the file.The honest tradeoff: optimized builds compile slower, and that is now the bottleneck — a cold-cache run is 21.5 min compiling vs 9.6 min testing. So this also sets
cache-workspace-crates: trueon rust-cache, which the wasm job already uses; the ~90 vcad crates were being rebuilt every run. Total job time on the cold-cache run was 33 min; the warm-cache steady state should be well below that.m3_flywheel_optimizeis marked a perf canary. It passes — but 250 L-BFGS iterations x 5 booleans is ~13 min locally, and--no-fail-fastmeans CI would now reach it for the first time. Revert the#[ignore]if you'd rather pay it.Verification
Full workspace exactly as CI runs it: 3926 passed, 0 failed, 61 ignored.
cargo fmt --all --checkclean. All 16 CI checks green, including the torture track (which byte-compares against an x86_64 baseline — good evidenceopt-level = 2does not perturb kernel geometry).🤖 Generated with Claude Code