Repository navigation
port: blend.fillet_solver_p3 - #236
denelson1-dot wants to merge 68 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reconciles tangency-labs#192 with tangency-labs#147 and with main's surface.offset_surface port (fillet_geom_p1 reads the solver through FilletSolver; OffsetSurface accessors; mutable LoadInitialValues surface), following tangency-labs#212's resolution of the same pair. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…set surface OffsetSurface::new takes the kernel and singularities (SM_SS_UNKNOWN, as the shim's default), offset_distance() is a method, test_analytic_surface takes the kernel. fillet_solver_p1, fillet_geom_p1 and fillet_custom_solver difftests: 0 failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ocessFullRollOver and helpers (WIP) GetOtherFilletCorner, sm_FindFilletTsectPnt, SplitAndTrimFilletSurface, StepSolve, sm_RefineRailCurveClassification, sm_CalcRailXEdge, ProcessFullRollOver. FilletSolverOps gains GetSolverType and the SmConstantDistanceFS::GetDistance read; FilletBrep::set_vertex_point is public. Calls into fillet_solver_p2, fillet_geom_p5 and fillet_standard_solver_p1 are UNPORTED stubs. Difftest to follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rnel tests The shim derives a deterministic SmFilletSolver (offset/offset intersection equations, configurable solver type, extra unknown, failing LoadInitialValues) mirrored by the runner; cases step along floor/wall fillets across domains, boundaries and solver types. 400 cases: 378 passed, 22 deferred (PointOnPlaneSolve, blend.fillet_solver_p2), 0 failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…findings, UB-175/176 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tangency-labs#192 now carries its own reconciliation with main's offset surface; its files are taken as is, plus this unit's FilletSolverOps::solver_type / constant_distance and the custom solvers' GetSolverType stub. sm_TestManifoldTangentRollover takes the kernel and a mutable B-rep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gency-labs#192 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ing tangency-labs#192 Same reconciliation as tangency-labs#236 (bf8411d): ConstantRadiusAssistedFs now wraps a FilletSolver, so fillet_geom_p1 reaches its surfaces, edgeuses and approx tolerance through .solver. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…et edges, ReCalcFilletGeom and TopologySplit Ports SmFilletVertex::CalcCornerVertGeom/CalcMateGeom, SmFilletEdge::CalcCornerEdgeGeom/CalcFilletIntFillet and SmFilletGeom::ReCalcFilletGeom/TopologySplit into blend::fillet_geom::p3, plus the co-ported SmFilletGeom(pSolver, type) for a base solver. Adds the C++ shim, FFI binding, difftest runner and fixtures, unit tests (TopologySplit is kernel-tested only), and upstream issues UB-175/UB-176. set_vertex_point in p1 is made pub, as in tangency-labs#236. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…unit/blend.fillet_solver_p3 Conflicts: both sides appended to plan/findings.md and plan/upstream-issues.md (kept both, UB entries in number order), and to the query.face_p2 caller note in surface/surface/p4.rs (kept both callers). main's heal.stitch_p1 took UB-176, so this branch's SplitAndTrimFilletSurface defect is renumbered UB-183 (UB-178..UB-182 are taken by open PRs). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gency-labs#214, tangency-labs#234) into unit/blend.fillet_solver_p3 Conflict: plan/upstream-issues.md, where main's UB-165 and this branch's UB-166 were added at the same place; kept both, in number order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bherbruck
left a comment
There was a problem hiding this comment.
Thanks for this port. I reviewed head b419ae4a against SmFilletSolver.cpp. The port is faithful and every gate I ran passed, so only one small formatting item stands in the way.
What passed
cargo fmt --check, clippy-D warnings(plain and with--features solidcore-ffi/cpp,difftest/cpp),cargo test -p solidcore(1399 passed, 0 failed) and the wasm32 check.--unit blend.fillet_solver_p3gives the same result as your PR description. The green set gives 212 units, 113 passed, 99 partial, 0 failed. It was run in two batches because one run hit my local time limit.git merge-treeagainstorigin/main(b6b6f7dd) is clean. Main has noUNPORTED(blend.fillet_solver_p3)stubs left to wire.- I ran 157 extra adversarial
StepSolvecases against the C++ as a local fixture: 150 passed bit-identical (max_rel_err 0), 7 deferred atPointOnPlaneSolve, 0 failed, and there were no crashes or rejections. They cover:- every accepted solver type, including
SurfSurfandCurveBased, which the generator does not pick; - real
SmPlanebases, both planes or mixed with a B-spline; - parallel and coincident surfaces;
- radius 0, negative and 1e-12; orientations 0, -1 and 2;
- point-sized and line-sized domains; current points outside the domain; extended domains with and without
allow_ext; - steps of ±1e300, 1e-300, -0.0 and a denormal; tolerances of 1e3, 1e-15 and 1e-300;
- scales of 1e-6, 1e6 and 1e9;
- constant-distance targets of 0, negative, 1e6 and 1e-12;
- zero, huge and tiny tangents;
- the extra unknown combined with
init_failfor each solver type; - starts on a boundary or 1e-13 inside it; a corner clip; degree-3/2 patches; angles of 20° and 170°.
- every accepted solver type, including
- Faithfulness checks that held up:
StepSolve: both clip tests, therdStepvsdStepSizeuse, theSetupOffsetValuesstatus being ignored,sUVwritten back from the EFO, the retry with an adjusting plane, and theSM_MAX_USER_DOUBLESguard.SplitAndTrimFilletSurface: thebFound1/2cross-setting andSM_BT_NONE'si > 0break.sm_RefineRailCurveClassification: the reversediiloop and theSEonRailEdgeuseIntersectand the drops.sm_CalcRailXEdge: the nine guesses.ProcessFullRollOver:IsConvexRadialSector(10)defaults, the loop walk andSetDataSize.Unitize'sSM_ERR_INVALID_INPUT(1007) matches the RustInvalidInputmapping.
- The FFI guard rejects only the variable-radius cast (undefined in the shim), a tolerance ≤ 0, unordered domains and malformed blocks.
UNPORTEDstubs inp3.rstarget onlyblend.fillet_solver_p2,blend.fillet_geom_p5andblend.fillet_standard_solver_p1. All three aretodoon main and tagged with the right owners.
PARTIAL blend.fillet_solver_p3 cases=400 passed=378 failed=0 deferred=22 max_rel_err=1.8011683050692552e-15 with_values=378 no_values=0 blocked_by=blend.fillet_solver_p2(22)
PARTIAL blend.fillet_solver_p3 cases=157 passed=150 failed=0 deferred=7 max_rel_err=0e0 with_values=150 no_values=0 blocked_by=blend.fillet_solver_p2(7) # extra adversarial fixture (local only)
PARTIAL blend.fillet_solver_p1 cases=333 passed=280 failed=0 deferred=53
PARTIAL blend.fillet_geom_p1 cases=461 passed=429 failed=0 deferred=19 rejected=13
PARTIAL blend.fillet_custom_solver cases=448 passed=401 failed=0 deferred=47
--green: 212 units, 113 passed, 99 partial, 0 failed
Must-fix
-
rustfmt on the
include!d unit files.cargo fmtdoes not reach these files, so CI misses them, butrustfmt --edition 2021 --checkreports diffs in both new files:crates/difftest/src/units/blend_fillet_solver_p3.rs: lines 23, 330, 339, 390, 437, 453, 466, 537 and 563 (longuselists, theOffsetSurface::newmatch arm,next_ud);crates/solidcore-ffi/src/units/blend_fillet_solver_p3.rs:73(the variable-radiusbad(..)message).
Fix: run
rustfmt --edition 2021on both files and commit the result.
Should-fix (short)
- The branch carries an older copy of #147. #147's current head (
b396cdd3) has since wired theUNPORTED(topology.brep_p1)stubs infillet_geom/p1.rs, andtopology.brep_p1is nowpartialon main. A trial merge ofb396cdd3into this branch conflicts incrates/solidcore/src/surface/surface/p4.rs. Re-merging the latest #147 head before you mark this ready would keep the dependency stack in sync. - Optional: one or two
SrfIn::Planebases in the generator would keep the analytic offset path compared in the committed cases. My plane cases all passed.
🤖 Generated with Claude Code
…iles cargo fmt does not reach the include!d unit files; review of b419ae4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflict: the query.face_p2 caller note in surface/surface/p4.rs; both sides list the same callers, tangency-labs#147's wording taken. Review of b419ae4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflict: plan/findings.md, where tangency-labs#192 moved and revised (round 3) its fillet_solver_p1 sections; tangency-labs#192's copy kept, this branch's older copy dropped, the fillet_solver_p3 section kept and its tangency-labs#192 head updated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
and tangency-labs#192 heads Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…to unit/blend.fillet_solver_p3 Conflict: plan/findings.md, where main's topology.brep_p10 section and this branch's fillet_solver_p1 sections were appended at the same place; kept both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the thorough review and the adversarial fixture. Must-fix: done. Should-fix: done. I merged both dependency heads again:
Gates at Optional |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…p2); trim kernel keeps caller rng; runner doc Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…merge_p3/p4, brep_p7 follow-up) into blend.fillet_solver_p1 findings.md: both sides kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t; offset.offset_geometry_creation perturb 431/431 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks for the round-3 changes, @denelson1-dot. I reviewed head 85cfe04c (it moved from ff0f701b while I worked; the only change is the #283 main merge). p3.rs's StepSolve change is right, and the intersector now reaches it. One thing has changed since my last review: blend.fillet_standard_solver_p1 (#168) is now on main as partial, and this branch has merged it. That turns the deferral we expected into stubs that can be wired, so there is one more round of work below.
Earlier items
- Must-fix (wire
UNPORTED(blend.fillet_solver_p3)in the intersector): fixed.FilletSolver::step_solvenow writes the flags to a caller'sStepSolveOutcomeand keeps them on an error.ConstantRadiusAssistedFs::step_solve(fillet_intersector.rs:496) passesselfas the solver and copies the flags back, asSmFilletIntersector.cpp:427-436reads them. The runner and shim now compare the three flags after a failed solve. The new kernel test covers this too. - Should-fix (findings entry for
allow_ext): done, with a reproducer. - Nit (#212's
UNPORTED(blend.fillet_solver_p3)stubs): still open on #212; whichever PR lands second wires them.
What passed
cargo fmt --all --check,rustfmt --edition 2021 --checkon the p3/p1 runner and FFI unit files and the intersector runner, clippy-D warnings(plain and with--features solidcore-ffi/cpp,difftest/cpp),cargo test -p solidcore(1515 passed, 0 failed) and the wasm32 check.git merge-treeagainstorigin/main(3132491e) is clean.- Extra cases against the C++, no crashes, panics or timeouts:
--seeds 3000onblend.fillet_solver_p3: 2849 passed, 151 deferred (PointOnPlaneSolve), 0 failed, max_rel_err 4.4e-15.--fuzz 2000onblend.fillet_solver_p3: 487 rejected, 652 ran, 0 differs.--fuzz 600onblend.fillet_intersector: 119 rejected, 183 ran, 0 differs.
- A local trial of the must-fix below. I wired the overrides and fixed the runner's write-back in my worktree only (nothing was pushed). After that,
blend.fillet_intersectorgave 967 passed, 0 failed, 319 deferred (now 934/0/352). All 94blend.fillet_standard_solver_p1deferrals go away, and every comparednext(StepSolve) case is bit-identical.
PARTIAL blend.fillet_solver_p3 cases=400 passed=378 failed=0 deferred=22 max_rel_err=1.8011683050692552e-15 with_values=378 no_values=0 blocked_by=blend.fillet_solver_p2(22)
PARTIAL blend.fillet_solver_p3 cases=3000 passed=2849 failed=0 deferred=151 max_rel_err=4.369504743270105e-15 blocked_by=blend.fillet_solver_p2(151) # --seeds 3000 (local)
PARTIAL blend.fillet_intersector cases=1286 passed=934 failed=0 deferred=352 max_rel_err=0e0 blocked_by=blend.fillet_solver_p2(37),blend.fillet_solver_p4(59),blend.fillet_standard_solver_p1(94),curve.crv_on_surf(162)
PARTIAL blend.fillet_intersector cases=1286 passed=967 failed=0 deferred=319 max_rel_err=0e0 blocked_by=blend.fillet_solver_p2(37),blend.fillet_solver_p4(61),blend.fillet_standard_solver_p2(59),curve.crv_on_surf(162) # local trial of the must-fix
PARTIAL blend.fillet_solver_p1 cases=346 passed=339 failed=0 deferred=7 max_rel_err=0e0
PARTIAL blend.fillet_custom_solver cases=448 passed=401 failed=0 deferred=47 max_rel_err=0e0
PARTIAL blend.fillet_geom_p1 cases=480 passed=446 failed=0 deferred=21 rejected=13
PARTIAL blend.fillet_geom_p2 cases=350 passed=219 failed=0 deferred=131
PARTIAL blend.fillet_standard_solver_p1 cases=3839 passed=3786 failed=0 deferred=42 rejected=11
PARTIAL offset.offset_geometry_creation cases=861 passed=428 failed=0 deferred=433
FUZZ-PASS blend.fillet_solver_p3 cases=2000 rejected=487 malformed=861 ran=652 differs=0 crashes=0 panics=0 timeouts=0
FUZZ-PASS blend.fillet_intersector cases=600 rejected=119 malformed=298 ran=183 differs=0 crashes=0 panics=0 timeouts=0
difftest: 232 unit(s), 114 passed, 118 partial, 0 failed # --green
Must-fix
-
Wire the
ConstantRadiusAssistedFsoverrides toblend.fillet_standard_solver_p1, and add the missingsolver_type.impl FilletSolverOps for ConstantRadiusAssistedFs(crates/solidcore/src/blend/fillet_custom_solver.rs:1703, merged in from #192) has these stubs:UNPORTED(blend.fillet_standard_solver_p1)forGetJacobianSize(:1714),SetupOffsetValues(:1724),OffsetRadiiCanChange(:1738),GetType(:1748) andLoadJacobian(:1768).
That unit is now
partialon main, and all five exist as inherent methods or constants inblend/fillet_standard_solver/p1.rs(setup_offset_values:595,load_jacobian:640,jacobian_size:579,offset_radii_can_change:573,CONSTANT_RADIUS_ASSISTED_FS_TYPE:173). Today 94 intersector cases defer on them (AGENTS.md section 5, item 9). The same impl also has nosolver_typeoverride, so it falls through to the trait defaultFilletSolverType::Unknown. That contradicts its doc comment at:1702("none falls through to the base"). UpstreamGetSolverTypereturnsSM_FS_CONST_RADIUS(SmFilletStandardSolver.h:58) orSM_FS_CONST_RADIUS_ASSISTED(:112), andStepSolvebranches on it (p3.rs:404). It is masked only whileSetupOffsetValuesis a stub. Fix:- Delegate each override to the inherent method, as #285 does for the macro solvers. For example,
ConstantRadiusAssistedFs::setup_offset_values(self, tsect)writes*pOffsetDist.load_jacobianmapsJacobianLoadonto(rail_index, [surface1, surface2], &mut [surf1_offset, surf2_offset], x)and copies the offsets back. - Add
fn solver_typethat returns the type of the class the C++ side builds. The intersector shim builds a realSmConstantRadiusFS(blend_fillet_intersector.rsFFI header), so that isConstRadiusthere. When I triedConstRadiusAssisted, onenextcase went down the assisted branch and differed from the C++. - Runner:
crates/difftest/src/units/blend_fillet_intersector.rs:575-597(Delta) and:598-615(Values) compute into clones of the arena points and never write them back. Upstream fillsrTsectPnt/rDeltaPntby reference (SmFilletIntersector.cpp:87-134), so onceSetupOffsetValuesruns, 33deltacases fail on the point record (for examplegen0047:values.rec[24]: cpp=2 rust=0). After the call, writepa/pbback withfi.base.base.point_mut(id(*a))?, whatever the status, since C++ writes in place before anySER.
With those three changes the intersector gives 967/0/319 locally (the line above). Then update the
blend.fillet_intersectornotes inplan/modules.toml. If you would rather this land in #192 first and be merged here, that works too. Either way, this branch should not carry the stubs once it is ready.
Should-fix (short)
p3.rs:222and:514call a freePointOnPlaneSolvestub taggedblend.fillet_solver_p2(SmFilletSolver::PointOnPlaneSolve). Upstream it is virtual (SmFilletSolver.h:258), andSmConstantRadiusAssistedFSoverrides it (SmFilletStandardSolver.h:123, unitblend.fillet_standard_solver_p2). This is the solverStepSolvereaches it on. AFilletSolverOps::point_on_plane_solvehook would keep the override reachable, so that porting the base in p2 does not quietly run the base for an assisted solver. For now the stub is fine; only the dispatch matters.p3.rs:450:UNPORTED(blend.fillet_standard_solver_p1): SmVariableRadiusFS::GetOriginalCurvenow names apartialunit.CurveBasedFs::original_curveexists there, butFilletSolverOpshas no way to reach it. Either add a small hook (anoriginal_curve()that isInvalidInputby default), or retag the stub with a note that it needs that glue, so the stale-stub audit does not count it as wire-now.
🤖 Generated with Claude Code
…ndard_solver_p1; PointOnPlaneSolve hook Review of 85cfe04. - ConstantRadiusAssistedFs's FilletSolverOps overrides (GetJacobianSize, SetupOffsetValues, OffsetRadiiCanChange, LoadJacobian, GetType) call the blend.fillet_standard_solver_p1 ports (tangency-labs#168) instead of returning Unported, and it gains GetSolverType. They answer for SmConstantRadiusFS, the class the intersector shim builds: SM_FS_CONST_RADIUS and SmConstantRadiusFS_TYPE. - blend.fillet_intersector runner: the Delta / Values ops write their points back to the arena, as upstream fills rTsectPnt / rDeltaPnt by reference before any SER exit. The intersector: 1286 cases, 967 passed, 0 failed, 319 deferred (its 94 _standard_solver_p1 deferrals: 33 pass, 59 defer at _standard_solver_p2 LoadInitialValues, 2 at _p4). - FilletSolverOps::point_on_plane_solve: PointOnPlaneSolve is virtual upstream (SmConstantRadiusAssistedFS overrides it), so StepSolve and sm_FindFilletTsectPnt dispatch through the hook; its default is the base's stub (blend.fillet_solver_p2). - The variable-radius GetOriginalCurve and constant-distance GetDistance stubs are justified (findings.md): those solvers do not implement FilletSolverOps yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tangency-labs#252 blend.fillet_geom_p4) into unit/blend.fillet_solver_p3 Conflict: the blend.fillet_solver_p1 notes. main carries tangency-labs#192's older text with one added parenthetical (the offset surfaces SetSurfacesToZeroOffset walks are ConstantRadiusAssistedFs::solver.surfaces); tangency-labs#192's newer notes are kept with that parenthetical added at the same place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tangency-labs#192 now wires the constant-radius solver's SmConstantRadiusFS overrides to blend.fillet_standard_solver_p1 itself, re-exports blend.fillet_geom_p1's FilStatus, and writes the intersector runner's Delta/Values points back. Where that overlaps this branch's round-4 wiring, tangency-labs#192's side is taken (fillet_custom_solver.rs, fillet_intersector.rs, the intersector runner, fillet_geom/p1.rs, its findings sections and notes). Kept from this branch: GetSolverType = SM_FS_CONST_RADIUS on ConstantRadiusAssistedFs (the class the intersector shim builds), the PointOnPlaneSolve hook, the justified GetOriginalCurve / GetDistance stubs, the step_solve override and its test, the intersector notes with the combined counts, and main's DISPATCH GAP note in the blend.fillet_solver_p1 notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nit/blend.fillet_solver_p3 Conflict: plan/findings.md, where main's query.face_p4 section and this branch's blend.fillet_solver_p3 section were appended at the same place; kept both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
, tangency-labs#300, tangency-labs#302) into unit/blend.fillet_solver_p3 Conflicts: plan/findings.md (main's surface.cone_p3 / cone_p2 section and this branch's blend.fillet_solver_p3 section, both kept) and the generated offset.offset_geometry_creation report (main's taken; regenerated by --green). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
--green at fb9e85e (x86_64): 234 units, 114 passed, 120 partial, 0 failed. With tangency-labs#192's SmCircularCrossSectionFSG::CreateSurfaceFromCurves wiring: 431 passed, 0 failed, 430 deferred (main's report: 428 / 433; its 4 blend.fillet_standard_solver_p1 deferrals: 3 pass, 1 defers at curve.crv_on_surf), as tangency-labs#192's own report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks for round 4, @denelson1-dot. I reviewed head 07f39394. The earlier must-fix is done, and the overrides behave as intended against the C++. One small item is left, below.
Earlier items
- Must-fix (wire
ConstantRadiusAssistedFs's overrides toblend.fillet_standard_solver_p1, addsolver_type, write the runner'sDelta/Valuespoints back): fixed in0487f538.jacobian_size,setup_offset_values(including*pOffsetDist),offset_radii_can_change,type_codeandload_jacobian(offsets copied back on an error too) delegate to the #168 ports.solver_typeisConstRadius, which matches theSmConstantRadiusFSthe shim builds. The runner writespa/pbback beforest(o, r). The intersector is at 967 / 0 / 319, the same as my local trial. - Should-fix (
PointOnPlaneSolvedispatch): done.FilletSolverOps::point_on_plane_solveis a hook, and bothStepSolveandsm_FindFilletTsectPntgo through it. - Should-fix (
GetOriginalCurvestub atp3.rs:446): done. It now has a justification, with a findings entry explaining thatCurveBasedFsis not on the trait yet.
What passed
cargo fmt --all --check, andrustfmt --edition 2021 --checkon the p3/p1 runner and FFI unit files, the intersector runner and the standard_solver_p1 runner.- clippy
-D warnings, both plain and with--features solidcore-ffi/cpp,difftest/cpp. cargo test -p solidcore(1534 passed, 0 failed) and the wasm32 check.git merge-treeagainstorigin/main(bb137d7d) is clean.- About 5,600 extra cases against the C++, with no crashes, panics or timeouts:
--seeds 3000on p3: 2849 passed, 0 failed.--fuzz 2000on p3: 0 differs.--fuzz 600on the intersector: 0 differs.
PARTIAL blend.fillet_solver_p3 cases=400 passed=378 failed=0 deferred=22 max_rel_err=1.8011683050692552e-15 with_values=378 no_values=0 blocked_by=blend.fillet_solver_p2(22)
PARTIAL blend.fillet_solver_p3 cases=3000 passed=2849 failed=0 deferred=151 max_rel_err=4.369504743270105e-15 blocked_by=blend.fillet_solver_p2(151) # --seeds 3000 (local)
PARTIAL blend.fillet_intersector cases=1286 passed=967 failed=0 deferred=319 max_rel_err=0e0 blocked_by=blend.fillet_solver_p2(37),blend.fillet_solver_p4(61),blend.fillet_standard_solver_p2(59),curve.crv_on_surf(162)
PARTIAL blend.fillet_solver_p1 cases=346 passed=339 failed=0 deferred=7 max_rel_err=0e0
PARTIAL blend.fillet_standard_solver_p1 cases=3839 passed=3786 failed=0 deferred=42 rejected=11
PARTIAL blend.fillet_custom_solver cases=448 passed=401 failed=0 deferred=47 max_rel_err=0e0
PARTIAL blend.fillet_geom_p1 cases=480 passed=446 failed=0 deferred=21 rejected=13
PARTIAL blend.fillet_geom_p2 cases=350 passed=219 failed=0 deferred=131
PARTIAL offset.offset_geometry_creation cases=861 passed=431 failed=0 deferred=430
FUZZ-PASS blend.fillet_solver_p3 cases=2000 rejected=487 malformed=861 ran=652 differs=0 crashes=0 panics=0 timeouts=0
FUZZ-PASS blend.fillet_intersector cases=600 rejected=119 malformed=298 ran=183 differs=0 crashes=0 panics=0 timeouts=0
difftest: 234 unit(s), 114 passed, 120 partial, 0 failed # --green
Must-fix
crates/solidcore/src/blend/fillet_custom_solver.rs:1629-1637: return the type instead of stubbing it. Thefillet_solver_ops!macro (used forEiffelTowerFsandSecantConstantRadiusFs) has asolver_typeoverride that returnsUNPORTED(blend.fillet_standard_solver_p1): SmConstantRadiusAssistedFS::GetSolverType. This branch adds that stub; it is not on main.blend.fillet_standard_solver_p1ispartialon main, and the upstream body is one inline line:return SM_FS_CONST_RADIUS_ASSISTED;(SmFilletStandardSolver.h:112). Both classes inherit it, becauseSmEiffelTowerFSandSmSecantConstantRadiusFSderive fromSmConstantRadiusAssistedFSand do not override it (SmFilletCustomSolver.h:27,:105). That makes this a stub into a ported unit (AGENTS.md section 4, step 5, item 9). It would also sendStepSolve(p3.rs:400) to an error where upstream takes the assisted branch.- Fix:
fn solver_type(&self) -> Result<FilletSolverType> { Ok(FilletSolverType::ConstRadiusAssisted) }, and update the doc comment. - I tried this locally.
cargo test -p solidcorestill passes (1534), andblend.fillet_custom_solver(401/0/47),blend.fillet_solver_p3(378/0/22) andblend.fillet_intersector(967/0/319) do not change. No compared path reaches this method today, so it is a one-line change with nothing to re-baseline.
- Fix:
Note (not blocking)
- #192 has moved to
f676c2e8since thee6f4d026you merged, andgit merge-treeof that head with this branch now conflicts. Whichever of the two lands second will need to resolve it. No action is needed here unless you want to pick it up now.
🤖 Generated with Claude Code
…_CONST_RADIUS_ASSISTED SmEiffelTowerFS and SmSecantConstantRadiusFS derive from SmConstantRadiusAssistedFS and inherit its one-line GetSolverType (SmFilletStandardSolver.h:112); the fillet_solver_ops! override returned an UNPORTED(blend.fillet_standard_solver_p1) stub into a ported unit. No compared path reaches it: blend.fillet_custom_solver 401/0/47, blend.fillet_solver_p3 378/0/22 and blend.fillet_intersector 967/0/319 are unchanged. Review of 07f3939. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
, tangency-labs#314) into unit/blend.fillet_solver_p3 Conflicts in offset.offset_geometry_creation: main's batch wired its topology.brep_p2 stubs (tangency-labs#149), and this branch carries tangency-labs#192's wiring of SmCircularCrossSectionFSG::CreateSurfaceFromCurves. The module's stub list now names only the curve.crv_on_surf stub left; the unit's notes take main's text (the tangency-labs#149 re-runs) and the report main's copy, both refreshed after --green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…with tangency-labs#192's wiring --green at 1ef8cb7 (x86_64): 235 units, 114 passed, 121 partial, 0 failed. main's topology.brep_p2 wiring (tangency-labs#149) plus tangency-labs#192's SmCircularCrossSectionFSG::CreateSurfaceFromCurves: 562 passed, 0 failed, 299 deferred (main's report: 552 / 309; its 11 blend.fillet_standard_solver_p1 deferrals: 10 pass, 1 reaches curve.crv_on_surf). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
blend.fillet_executive_p2) into unit/blend.fillet_solver_p3 Conflicts: the blend.fillet_standard_solver_p1 module table (tangency-labs#192's rows for the reconciled types, main's FilletLaw row) and plan/findings.md (main's blend.fillet_executive_p2 section and this branch's blend.fillet_solver_p3 section, both kept). Fallout of tangency-labs#192's newer generator variants: blend.fillet_executive_p2 builds the circular and blend-curve fillet surface generators through FilletSurfaceGenerator::circular / blend_curve (the variants wrap blend.fillet_standard_solver_p1's CircularCrossSectionFsg / BlendCurveCrossSectionFsg; continuity is u32). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
, tangency-labs#286, tangency-labs#298) into unit/blend.fillet_solver_p3 Conflicts: plan/findings.md (main's curve.curve_p8 section and this branch's blend.fillet_solver_p3 section, both kept), the offset.offset_geometry_creation notes (main's rewrite after tangency-labs#286 taken; the tangency-labs#192 wiring sentence is re-added with fresh counts after --green) and its generated report (main's taken; regenerated by --green). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…with tangency-labs#192's wiring --green at 8890f8a (x86_64): 237 units, 115 passed, 122 partial, 0 failed. With tangency-labs#192's SmCircularCrossSectionFSG::CreateSurfaceFromCurves wiring: 874 cases, 575 passed, 0 failed, 299 deferred (main's report: 565 / 309; its 11 blend.fillet_standard_solver_p1 deferrals: 10 pass, 1 reaches curve.crv_on_surf). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tch) into unit/blend.fillet_solver_p3 main's tangency-labs#351 wires stale stubs in blend.fillet_custom_solver and blend.fillet_solver_p1 on main's older tangency-labs#192 (a88c62a); this branch carries tangency-labs#192's newer head (e6f4d02), which wires the same ones (the macro solvers' inherited SmConstantRadiusFS overrides, SmLinearCrossSectionFSG::CreateSurface, sm_TestAnalyticSurface's cone test). Those hunks keep this branch's side (equivalent; it also keeps upstream's failing iso curve status read as TRUE); main's now unused SurfaceKind import is dropped. plan/findings.md keeps both sides' sections; the blend.fillet_solver_p1 report keeps this branch's (339 / 7, as --green gives). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for round 5, @denelson1-dot. I reviewed head Earlier item
What passed
Nit (not blocking, can be a follow-up)
🤖 Generated with Claude Code |
, tangency-labs#239) into unit/blend.fillet_solver_p3 Conflict: the offset.offset_geometry_creation notes (main's topology.face_p4 sentence and this branch's tangency-labs#192 one were appended at the same place; main's taken, this branch's re-added with fresh counts after --green) and its generated report (main's taken; regenerated by --green). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…with tangency-labs#192's wiring --green at d063188 (x86_64): 239 units, 116 passed, 123 partial, 0 failed. With tangency-labs#192's SmCircularCrossSectionFSG::CreateSurfaceFromCurves wiring: 874 cases, 595 passed, 0 failed, 279 deferred (main's report: 585 / 289; its 11 blend.fillet_standard_solver_p1 deferrals: 10 pass, 1 reaches curve.crv_on_surf). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dispatch, tangency-labs#317, tangency-labs#356, tangency-labs#361, tangency-labs#414) into unit/blend.fillet_solver_p3 Conflicts: tangency-labs#313 threads &Kernel through the Surface / Curve evaluations; this branch's side is kept where it carries tangency-labs#192's newer head (FilletGeomRef intersector tests, rail / rail_mut accessors, the wired VariableRadiusFs::set_extension_flags, the iso curve status read as TRUE), with main's kernel argument added (set_extension_flags now takes the kernel and the B-rep), main's new SurfaceKind::Cone arm, and main's constructor order in the intersector runner; findings keep both sides' sections, the blend.fillet_solver_p1 notes keep tangency-labs#192's newer text. Fallout in code main does not carry: blend.fillet_solver_p3 passes the kernel to its evaluations (offset_normal takes it), and the local solvers' function interfaces, which pass none, see FindClippedRadiusEfo / FilletSphereSolveEnfo through ClippedRadiusFn / SphereSolveFn adapters that hold it; tangency-labs#192's newer ConstantRadiusAssistedFs::load_jacobian, compute_surface_values call and RailUvCurveIntersectEnfo, and the p3 runner's harness solver, take the kernel too. --perturb on blend.fillet_solver_p3: 378/378 detected; difftest runner tests pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…(kernel-aware Surface dispatch) --green at 00eacf1 (x86_64): 239 units, 119 passed, 120 partial, 0 failed; blend.fillet_solver_p1 346 / 339 / 7, unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…angency-labs#281) into unit/blend.fillet_solver_p3 Conflict: the offset.offset_geometry_creation notes; main's issue tangency-labs#76 link and this branch's tangency-labs#192 sentence are both kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gency-labs#415, tangency-labs#353, tangency-labs#405, tangency-labs#407) into unit/blend.fillet_solver_p3 Conflict: plan/findings.md, where main's topology.brep_p6 section and this branch's blend.fillet_solver_p3 section were appended at the same place; kept both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ports unit
blend.fillet_solver_p3(SmFilletSolver.cpppart 3) intoblend::fillet_solver::p3:FilletSolver::step_solveSmFilletSolver::StepSolveFilletSolver::split_and_trim_fillet_surfaceSmFilletSolver::SplitAndTrimFilletSurfaceFilletSolver::process_full_roll_overSmFilletSolver::ProcessFullRollOverFilletSolver::other_fillet_cornerSmFilletSolver::GetOtherFilletCornerfind_fillet_tsect_pntsm_FindFilletTsectPntrefine_rail_curve_classificationsm_RefineRailCurveClassificationcalc_rail_x_edgesm_CalcRailXEdgeStatus: partial. It cannot merge before its dependencies.
Dependencies (not re-ported)
This branch builds on #147 (
blend.fillet_geom_p1, headb396cdd3, now on main through #156) and merges the head branch of the open dependency PR #192 (blend.fillet_solver_p1, heade6f4d026; main carries its older heada88c62afthrough #256), per AGENTS.md §4. #192 now carries its own reconciliation with main's offset surface, so its files are taken as they are. With both PRs merged in, the affected difftests have 0 failures.Changes outside the unit
FilletSolverOpsgainssolver_type(GetSolverType, pure virtual upstream) andconstant_distance(theSmConstantDistanceFS::GetDistanceread inSmFindClippedRadiusEFO). The custom solvers'GetSolverTypeoverride isUNPORTED(blend.fillet_standard_solver_p1).FilletBrep::set_vertex_pointis now public.blend.fillet_intersector: main'sUNPORTED(blend.fillet_solver_p3)StepSolveback-edge is wired.FilletSolverOps::step_solveis now required, andConstantRadiusAssistedFscallsFilletSolver::step_solvewith itself as the solver, so its own overrides are kept. Of the intersector's 120 cases deferred here, 59 now pass and 61 deferred one level deeper atblend.fillet_standard_solver_p1; with the overrides below wired, the intersector is at 1286 cases, 967 passed, 0 failed, 319 deferred.FilletSolverOpsoverrides (GetJacobianSize,SetupOffsetValues,OffsetRadiiCanChange,LoadJacobian,GetType) callblend.fillet_standard_solver_p1(port: blend.fillet_standard_solver_p1 #168). port: blend.fillet_solver_p1 #192 wires these too, and its version is the one merged here. This unit addsGetSolverType, which isSM_FS_CONST_RADIUS: one Rust type stands for bothSmConstantRadiusFSandSmConstantRadiusAssistedFS, and the intersector shim builds aSmConstantRadiusFS. WithSM_FS_CONST_RADIUS_ASSISTED, a clipped step would take the assisted branch, which the C++ doesn't. The custom solvers derived fromSmConstantRadiusAssistedFS(SmEiffelTowerFS,SmSecantConstantRadiusFS) inherit itsGetSolverType,SM_FS_CONST_RADIUS_ASSISTED. The intersector runner writes itsDelta/Valuespoints back, since upstream fills them by reference. The intersector's 94_standard_solver_p1deferrals: 33 pass, 59 defer at_standard_solver_p2(LoadInitialValues) and 2 at_p4.PointOnPlaneSolveis aFilletSolverOpshook, because it is virtual upstream andSmConstantRadiusAssistedFSoverrides it. Its default is the base's stub (blend.fillet_solver_p2). The variable-radiusGetOriginalCurveand constant-distanceGetDistancestubs are marked justified: those solvers don't implementFilletSolverOpsyet (seeplan/findings.md).FilletSolver::step_solvewrites its three flags to aStepSolveOutcomethe caller passes in, so a failure keeps what was set before it (upstream'sSmFilletIntersectorreadsbClipped/bBoundaryHitafter a failed solve). The shim and runner compare the flags after a failed solve too.Stubs
Calls into units that are not ported yet go through
UNPORTEDstubs:blend.fillet_solver_p2:PointOnPlaneSolve,RailEdgeuseIntersect,GetFilletCorner,CreateCliffRollOverSideFGblend.fillet_geom_p5:MakeFilletEdge,TrimCliffRailsblend.fillet_standard_solver_p1: theSmVariableRadiusFScast, andGetDistanceby defaultDifftest
Every library solver belongs to an unported unit, so the shim derives a deterministic
SmFilletSolverand the runner mirrors it. The shim'sLoadJacobiansolves the offset/offset intersection; the shim also lets the test set the solver type, use one extra unknown, or makeLoadInitialValuesfail.StepSolvecases step along floor/wall fillets at 20–160° and scales from 1e-3 to 1e3, on flat and bumped B-splines. They cover:With main's
topology.face_p3, #147'sclone_for_evaluation(the caches keep the surface's owner, as upstream's pointer does) moves 46topology.face_p3cases (the Endcap intersect fixtures) from pass to deferred at main's existingsurface.surface_cache_p2stub forSmTol::GetZoneTol3don an opaque handle; main alone passes them. No failures; the regenerated reports are committed (146ecdd6).With
allow_ext = true, results on degenerate inputs (radius0or1e-12, coincident surfaces) are not bit-identical; withallow_ext = falseeverything agrees bit for bit. The difference is insurface.offset_surface's extension path, not inStepSolve;plan/findings.mdrecords it with a reproducer as a follow-up for that unit.Local verification: fmt and workspace clippy (with and without
cpp) are clean,cargo test -p solidcorepasses (1598 tests, 12 of them new for this unit), andwasm32-unknown-unknowncheck passes.The other six routines are covered by kernel tests only. On every path a native fixture can build, they reach
blend.fillet_solver_p2or fillet-geometry stubs. When p2 lands they should get difftest cases. Details are inplan/findings.md.Checklist (§4 step 5)
IsConvexRadialSector(10)passesbOptStepOff = FALSEanddTolDeg = 0;SmOffsetSurfaceusesSM_SS_UNKNOWN.StepSolve, andSM_BT_NONE's early break.SEstatuses are ignored,Unportedalways propagates, and the effect ofSetMin/Intersectfailures is reproduced.Also noted for #208: its
ProcessRollOverpassesopt_step_off = truetois_convex_radial_sector, but upstream uses the defaultFALSE.🤖 Generated with Claude Code