Conversation
raorjun
added this pull request to stack #65
September 23, 2026 01:34
The reverse lookup answers "prescribe target metrics, solve for the car". But it swept 23 parameters that mixed paddock-adjustable knobs with properties that are fixed once the car exists. A request for a setup spent the sample budget on axes the answer cannot act on. It also returned a car you would have to rebuild, not a setup sheet. Variables in vehicle_architecture.yaml now have a scope: tag. DOE_SCOPE (or BOBSIM_DOE_SCOPE) and the two wrapper targets select it. The split is in the YAML, and Python has no variable lists. An untagged variable belongs to every scope, so a scoped sweep never drops a new entry silently. The default scope is all, so existing commands do not change. The reverse lookup no longer reports unusable answers as answers. Four advisory guards print to stderr and do not change the returned row: - a target outside the sampled population, in population-widths, because a bare normalized distance of 4.35 reads like an error bar - a population of smoke-test size - a results table older than its inputs - a population narrower than the config claims The generated config records the scope. So the last guard still fires after _doe_config.yaml is restored from git. Without the record, the lookup reads 23 parameters from a 14-parameter population and gives no warning.
opt-search can return only a vehicle that the sweep sampled. The cost to sample a space well increases exponentially with each parameter. So opt-search is the wrong tool for "what do I set on this car to hit these numbers". opt-solve treats that question as a small bounded least-squares problem: 1. Run a star design of 2n + 1 simulations. 2. Fit a slope and a curvature for each knob. 3. Solve the inverse on that surrogate. 4. Simulate the proposed setup. 5. If the result misses, add the miss with a secant update and repeat. The solver reports only results that it simulated. Most evaluations need no compile. A compile is about 70 % of the wall time for a variant, and the equations do not change. So springs, bars and dampers go to one cached executable with -override. That matches a recompiled run to 2.5e-5 deg/g on understeer gradient. Toe and camber cannot use an override, and the failure is silent. They build the wheel's toHub.R_rel rotation matrix, which OpenModelica evaluates at compile time. The parameter still reports isValueChangeable, and the runner accepts the override. Every bound copy updates, but the matrix that the wheel uses does not change. Mass and CG values fail in the same way through combineMassRecords. So runtime overrides are an allow-list, and the solver compiles everything else. The solver writes its own test matrix into a config for each evaluation. It uses one isoline with six points in the linear fit band, not three. That reduced the understeer-gradient fit error from 1.4-6.1 % to 0.27 %, and the case count from 22 to 8. The shared standard and its baselines do not change.
CI type-checks with scipy-stubs installed. The stubs expose the real signature of least_squares, and mypy could not infer the inline lambda against it. A local run without the stubs treats scipy as untyped and passes, so the error got through. Verified inside the container, which has the pinned stubs.
A review of this branch on four areas (reuse, simplification, efficiency, altitude) found one defect. The evaluator rewrote the sweep's committed _doe_config.yaml at scope `all`. After a scoped sweep, that silently erased the scope that opt-search needs to say which population it reads. The solver now generates its own copy next to its caches and does not touch the sweep's copy. Targets can be in solve_config.yaml, so a bare `make opt-solve` runs from the file. TARGETS= replaces the targets. It does not merge them. The config drops from 94 lines to the five items a person sets: targets, knobs, tolerances, the test matrix and a CPU count. - regularization, the star step and the verification budget were solver internals. Their defaults were in three places and did not agree: solve() said 3, and the YAML and CLI said 4. They are now constants in solver.py. - The runtime-override list is a fact about the model. It was in a file that said not to edit it. It is now in overrides.py, next to the evidence for it. - make could not reach --plan, --solve-dir or --config. They had no tests and no docs, so they are gone. - The 256-combination fallback is gone. This repo can reach 16. - The scaled-table override path is gone. Only aero uses it, and aero cannot be a knob. overrides.py now refuses a scalar with no start value. The runner finds names by start value and would drop that scalar silently. That is the failure the module exists to prevent. Its docstring also said that every swept parameter accepts an override, which this branch disproved. The hash-input tuple had five copies in compiler.py, build_pipeline.py and the evaluator. It is now one constant. steady_state_eval_report takes explicit isoline, max_workers and render_report parameters, not an open-ended block-override hook. The solver skips the PDF that it never read. Measured on the real car: the solver skips the PDF, and a cached solve went from 67 s to 60 s. A cold solve was not faster in this run.
OptSim could sample a design space and invert for a setup. It could not answer the question that a design review asks: what does this specific change give, and what does it cost, across more than one study. opt-trade reads a YAML of named candidates and the metrics to compare. It compiles each candidate once and runs each requested standard against that one executable. Then it writes a comparison table. SteadyStateEval, RampSteerEval and TransientEval all run the same compiled model, and _3_StandardSim already builds it once for all three. They also share one interface. So pipeline/standards.py is a registry of three entries and a generic runner. The sweep's batch step now uses the registry. Before this change, a second standard in compiler_config.yaml would have run SteadyStateEval's report against the wrong executable. opt-trade compiles each candidate and does not use an override. opt-solve makes the opposite choice. A trade study must be able to change mass, CG, toe and camber, and an override of those silently does nothing. The solver's evaluator had a private cache of compiled vehicles. That cache is now pipeline/variants.py, and both tools share it. The cache key is the content, so two studies build and simulate a shared vehicle only once. The report has no score and no ranking. It makes sure that each difference is worth reading: - It shows a delta below the metric's stated resolution, but marks it. - It does not compare a run that lost simulation cases. - If one candidate is exactly two others combined, it reports the interaction. So "we'll do both" is not assumed to be the sum. TransientEval reports some metrics once per group under one name. You must ask for those by group. A metric that repeats with different values is an error. Real run on the example study, four vehicles by two standards: 1144 s cold, 3 s cached.
The trade study refused to compare a run that lost simulation cases, but the solver did not. A star point that lost a case still returns finite gradients, fitted through fewer points. So it bent the surrogate, and no number looked wrong. Evaluations are cached, so it would also have bent every later solve. Both tools now share one definition, standards.case_loss. The solver stops and reports the variant, the counts and what to change. This commit also replaces the error for tooling that changes during a run. The error came from the sweep's compiler and said to run make clean-opt. That advice is wrong here, because the store discards a stale cache by itself on the next run. A real run hit this error when another process edited hashed files between the star and the first verification compile. The first end-to-end solve over compile-only knobs found that problem. The solve now passes. Front toe, rear toe and the rear bar together built five executables in one batch, and the bar's star points shared the baseline executable. The verification compiled exactly one more. The solve converged on the first try at 0.3578 / 0.8359, against 0.35 / 0.85, in 617 s.
The routing table and the docs index still described OptSim as sweeps plus a reverse lookup. So an agent asked to size a bar or compare two options would go to the wrong tool. They now name all three tools and say which question each one answers. Three traps cost real time during this work. They are now under common mistakes: - OpenModelica accepts an override of toe, camber or any mass value, and the override silently does nothing. - A consumer must not rewrite the sweep's committed _doe_config.yaml. It records the scope that opt-search needs. - Git Bash on Windows rewrites the /workspace paths that make passes to Docker.
Comments and docstrings only. No code, value or key changes.
The previous commit trimmed comments only. It left the Markdown that this stack adds, and several error messages and docstrings, in long sentences with semicolons. - The OptSim sections of AGENTS.md, docs/doe-reverse-engineering.md, docs/architecture.md, docs/workflows.md and docs/README.md now use short sentences in active voice. Facts, numbers, links and anchors do not change. - Error messages and docstrings in solver.py, overrides.py, variants.py, generator.py and trade_study.py, and one skip message in test_doe_pipeline.py, no longer use semicolons. Text that a test matches does not change.
raorjun
force-pushed
the
chore/trim-comments-ste
branch
from
September 23, 2026 06:26
7187932 to
c4386e8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This PR cuts BobSim's comments and docstrings to the minimum that explains why something exists. It answers the review on #59: "Can cut out external info and justifications here." The same problem was in the whole repo, so this pass covers all of it.
The PR has two commits.
1. Cut comments to the minimum and write the rest in STE style. Comments and docstrings only.
vehicle_architecture.yamlis now one line:# Driver mass and CG change between sessions, so they are untagged and sweep in every scope.2. Write the docs and error messages that the stack adds in STE. The first commit did not include the Markdown that #59–#64 add, or some error messages and docstrings.
AGENTS.md,docs/doe-reverse-engineering.md,docs/architecture.md,docs/workflows.mdanddocs/README.mdnow use short sentences in active voice.solver.py,overrides.py,variants.py,generator.pyandtrade_study.pyno longer use semicolons. The same applies to one skip message intest_doe_pipeline.py. Text that a test matches does not change.134 files, +1108 / −5614. Stacked on #64.
How to check
ruff check .passes.python -m pytest testspasses with the BobLib submodule checked out. The 14 failures in the first version of this PR came from a worktree with no submodule, not from the code.Notes
mainafter Make BobVis the app's Replay tab #57, style: match the bobdyn.com theme in the _5_App browser UI #58, Add .dockerignore to shrink the docker compose build context #60, Make OMC build cflags architecture-aware for ARM64 hosts #62 and Run make app and make visual-* in Docker #67. Make BobVis the app's Replay tab #57 moved BobVis into the app and deleted six_1_VisualSimmodules and three tests. This PR no longer edits those files. In the files that Make BobVis the app's Replay tab #57, Make OMC build cflags architecture-aware for ARM64 hosts #62 and Run make app and make visual-* in Docker #67 changed,main's code is kept and only its comments are trimmed.left_PC/right_PC,_mech_trail_calculation(meters, not degrees), andRR_bump_spring_MR.Fr_stabar_MRjouncesRL_quarter_carto get its motion ratio.docs/in a follow-up.variants.pykept a few comments where the trim conflicted with the stack.