[1-Bort-1]

Until now a KPS3 or KPS4 run described its system only through a segment count and a few settings. Nothing that reads a system definition could draw it, KiteViewers included. Now the model can describe itself as a KiteGeometry system definition: its points, the springs it integrates as segments, one tether and one winch. That definition can be saved in the log, so a viewer reading the log gets the topology the run actually integrated. KiteGeometry is re-exported, as KiteModels already does for KitePodModels, WinchModels and AtmosphericModels.

Who this affects: a script that does using KiteModels together with using GLMakie or using GeometryBasics now finds Point ambiguous, and writes Point3f or KiteGeometry.Point instead. The examples never use a bare Point, and the changelog says so. calc_heading, which KiteGeometry also exports, stays KiteUtils' function for using KiteModels.

integrator = init!(kps4)
definition = system_definition(kps4)         # new
# ... simulate and log! ...
save_log(logger, definition, "sim_log")      # stores the definition in the log
KiteGeometry.definition(load_log("sim_log")) # reads it back

What the definition holds:

• Points. One point per model point, named 1 upwards. Point 1 is the winch and is static.
• Segments. For KPS4 these are the model's own spring list: 6 tether springs and 9 bridle springs, with their endpoints, rest lengths and diameters. The connectivity is therefore what the model integrates, not a second copy of the spring table. Of the 9 kite springs, the 3 that end at the KCU are named as bridle lines and the 6 that join two kite points as struts. All 9 are massless, as KPS4 treats them. They keep the line diameter KPS4 takes their stiffness and drag from, so the definition describes the system that was simulated. KPS3 has no spring list, so its segments join neighbouring points.
• Materials. Each segment names a linear material holding its density and Young's modulus, chosen so that the modulus times the line's cross-section gives the nominal stiffness the model integrates. KPS4 has exactly two, the tether and the massless kite lines, each read from the first spring of that kind. Comparing the springs' values would not work, because stiffness times length rounds differently for springs of different lengths. KPS3 has one, the tether.
• Masses. For KPS4 the KCU mass sits on the KCU point and the kite mass on points A–D. For KPS3 both sit on the last point. Tether mass comes from the tether material's density and the segments' diameter.
• Tether and winch. The tether runs from the winch to the KCU. Point 1, the winch, is the only fixed point. Gear ratio and drum radius come from the settings.
• Pose and stiffness. The pose is wherever the points are when the definition is built, so build it right after init!. The stiffness is the nominal value. The model only scales it below nominal during a start-up ramp.
• Log format. Saving and reading the definition in a log are KiteGeometry's, so every model that builds a definition logs it the same way. The log stores it as JSON under the key topology. The definition carries its connectivity checksum, so a reader can tell when a log's states were recorded against a different topology.

Where I'd push back:

• The example is not literally unchanged. The torque-control reel-out example changes two lines: it builds the definition after init! and passes it to save_log. Its logger never sees the model, and KiteUtils' logger has nowhere to keep metadata, so an untouched script cannot carry the definition without global state. The simulation itself is unchanged.
• The issue says this repo writes aero_force_b and tether_induced_force every step. It writes neither, so the KiteUtils state rename has nothing to change here.
• The pose is not moved to the zenith. By the awesIO convention a structure file places its kite at 90° elevation. This definition keeps the points where init! put them, so its positions match the first logged state. Rotating them would be one more step that a viewer would have to undo.
• The struts are labelled, not modelled. Giving them their own stiffness and diameter would change simulation results, so it belongs in a separate change to KPS4.

Dependencies: this needs KiteGeometry 0.3.2, the registered release, so CI installs it from the registry. 0.3 segments name a material rather than carry their own density and stiffness, and 0.3.1 added the keyword metadata constructor used here. 0.3.2 returns row names as symbols rather than strings, a change it shipped as a patch release. That is why the bound is 0.3.2. Both manifests are main's after a minimal resolve, and differ from main's only by the KiteGeometry entry. The Julia 1.13 manifest keeps main's Julia 1.13.3 standard-library entries, because the resolve ran on Julia 1.13.0.

Verification

• Reproduced first, on Julia, with the materials matched by value: 19 of 50 line diameters from 1.0 to 5.9 mm threw ArgumentError: two materials are named kite_line. At 3.0 mm the kite springs gave three unit stiffnesses: 388772.0908817369, 388772.09088173695 and 388772.09088173683.
• test/test-system-definition.jl sweeps those 50 diameters. Before the fix it stopped at 1.1 mm with 2 passes and 1 error. After it, all four testsets pass: 15, 100, 5 and 1 (Julia 1.13.0, examples project, with the file's own project switch removed). The file also checks fixed points, rest lengths, the stiffness from each material, the massless kite lines and the segment names.
• Acceptance, last run on e131bad and not repeated since: examples/reel_out_4p_torque_control.jl runs to the end and logs 1800 rows of 11 points. KiteGeometry.definition on the loaded log gives 11 points (length(kps4.pos) is 11) and 15 segments whose endpoints equal the model's springs (true), with the materials tether (724 kg/m³) and kite_line (0 kg/m³). The stiffness from the materials matches the springs at stiffness_factor 1.0, and the drum radius is 0.1615.
• Point next to GLMakie and GeometryBasics, on Julia 1.13.0: UndefVarError: Point not defined, with the hint that two modules export it. calc_heading under using KiteModels, KiteUtils is KiteUtils' function.
• Docs build clean on fe89ddf · up to date with main · REUSE lint: CI only (no reuse on the box).
• Local full suite and GitHub CI on fe89ddf: started, results are not in yet. Their state is the checks on this page and the local-suite comment on the thread. e7689e4, the commit before it, passed every GitHub check.
• Benchmark: the simulation step is untouched. On KiteGeometry 0.3.2, building the definition allocates 133 kB in 2385 allocations, once per run. Saving an 1800-step log allocates 14.06 MB instead of 13.92 MB and writes 340 kB instead of 329 kB. Timings not reported: the box was saturated (load 247 of 20 cores) and the benchmark check said to discard them.
• Risk: a definition built after the run has started records the current pose, not the initial one. The docstring says to build it after init!.

Opened by 1-Bort-1, an AI agent working for @1-Bart-1.
Closes #322 · task KiteModels.jl-322

[1-Bort-1]

Independent review (advisory)

Verdict: APPROVE WITH COMMENTS · 3 inline, 0 off the diff

Good

• Connectivity comes straight from s.springs, so there is no second copy of SPRINGS_INPUT, as the card says; checked against init_springs!/loop! in src/KPS4.jl
• KCU extra mass is set.kcu_mass and not s.masses[segments+1], so the tether half-masses that loop! adds at KPS4.jl:484 are not counted twice
• pos_ENU is an SVector field in KiteGeometry's Point, so the definition copies the pose and does not alias the live MVectors; confirmed in _structure.jl:73
• unit_stiffness = axial_stiffness * length gives the stiffness per length that the schema field asks for (N); matches the set.axial_stiffness / L_0 construction
• Both model methods share one 3-argument assembler, so there is one path from model to definition
• Deferring connectivity_sha and to_json to KiteGeometry and not adding a third preimage writer is the right call; the reason is in the card
• The round-trip testset protects the actual contract, a saved log parsing back to the same structure document

Not good

• src/system_definition.jl:17 — All 9 kite springs get d_line and rho_tether, but KPS4 puts no mass on them (the kite points already hold all of set.mass), and several are canopy struts, not lines. Anything that sums segment mass will read a heavier system than the one that was simulated.
• src/system_definition.jl:52 — A 7-argument positional Metadata(...) with two empty strings, a version literal and another empty string can't be reviewed from the diff. Keyword construction, or a KiteGeometry default, would name connectivity_sha and make the placeholder visible.
• test/test-system-definition.jl:31 — This compares an immutable snapshot with spring endpoints that loop! never changes, so it passes whatever the step does. The next_step! and the assertion add runtime but protect nothing beyond a check before the step.
• The [sources] pin to an unregistered KiteGeometry means ./bin/release cannot register KiteModels until the pin comes out; merging to main blocks releases until then
• @reexport using KiteGeometry is only in the changelog; the card never gives a reason for pushing Point, Segment, Tether, Winch, Metadata into every user's namespace
• The "1.0.0" and "structure_schema.yml" literals copy constants KiteGeometry owns and will drift when its schema version moves
• stiffness_factor is left out of unit_stiffness, so the example (stiffness_factor=0.1) logs 10x the stiffness it actually simulated; the docstring says nothing about this
• The 3-argument system_definition(s, segments, extra_masses) is a helper, but it becomes a documented method of an exported function
• Prose in docs/src/functions.md and the changelog is hard-wrapped, against §6

_{claude, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.}

[1-Bort-1]

Local full suite: PASS (17 min, Julia 1.13.0, one cell of the matrix)

[1-Bart-1]

Fix the failing test

[ufechner7]

Done. The save_log method and the log reader are in OpenSourceAWE/KiteGeometry.jl@b9583dd, whose pull request opens with this turn: save_log(logger, system, name) stores the definition, and KiteGeometry.definition(load_log(name)) reads it back. #327 switches to them in b44220a and drops its own helper. It is in draft until a KiteGeometry release carries the change.

What is the performance overhead by this change?

[ufechner7]

• src/system_definition.jl:17 — All 9 kite springs get d_line and rho_tether, but KPS4 puts no mass on them (the kite points already hold all of set.mass), and several are canopy struts, not lines. Anything that sums segment mass will read a heavier system than the one that was simulated.

Was this addressed? How could this be addressed?

[ufechner7]

The comments must be addressed.

[1-Bort-1]

All three are addressed. Bridle mass: f100446. Strut labels: 40e5ca0. Metadata by keyword: 40e5ca0, through OpenSourceAWE/KiteGeometry.jl@50b05ac (its pull request opens with this turn). Test without the no-op step: f100446. Each comment has a reply under it.

[1-Bort-1]

None per simulation step: the step code and the model are unchanged. The cost comes once, when you build the definition and when you save the log. Measured on KPS4 with system.yaml (Julia 1.13.0, BenchmarkTools medians, quiet box):

• Building the definition with system_definition(kps4): 178 µs and 141 kB, once per run.
• Saving an 1800-step log: 11.21 ms with the definition, against 11.26 ms and 10.96 ms without it (measured before and after), so within noise. It allocates 14.08 MB instead of 13.92 MB.
• Log file: 342 kB instead of 329 kB.
• using KiteModels: about 38 ms more out of about 2.2 s, for KiteGeometry and the JSON package it loads (@time_imports, one cold session).

The numbers are now on the Benchmark line of the description of #327.

[1-Bort-1]

Yes, both parts are now addressed.

• Mass, in f100446: the 9 kite springs have density 0, so summing segment mass gives only the tether. The kite mass sits on points A–D and the KCU mass on the KCU point, as in KPS4. The test asserts both.
• Struts, in 40e5ca0: the 3 springs ending at the KCU are named bridle_i and the 6 that join two kite points (A–B, A–C, A–D, B–C, B–D, C–D) strut_i. The test asserts the names and that there are 6 struts.

Their diameter and stiffness stay as KPS4 simulates them: all 9 use d_line (src/KPS4.jl:1138, :324). Giving struts their own properties would change simulation results, so that is a KPS4 change for a separate issue.

[1-Bart-1]

Review finding, which needs fixing before this merges.

line_material! in src/system_definition.jl compares materials with material.parameters == parameters, an exact float comparison on youngs_modulus. Each kite spring's value comes from (EA/l)*l, so springs of different lengths can land one ulp apart. The second value then adds another material with the same name, and SystemDefinition throws ArgumentError: two materials are named kite_line.

Replaying the arithmetic in Python floats, default settings pass, but 19 of 50 d_line values fail (3.0 mm among them), and so do 27% of random geometries. This is a replay, not an observed throw, so reproduce it in Julia first. The tests only use the default system.yaml, so CI cannot catch it. Add a test on a geometry that fails.

Also: the card cites an old commit (3b04585) with CI "running", though the head's checks are green, and its benchmark was measured on KiteGeometry 0.1. And unlike KiteViewers#66, neither the card nor the changelog warns that Point becomes ambiguous next to GLMakie now that KiteGeometry is re-exported.

Posted by the owner's Claude Code session, on the owner's instruction to help review.

[1-Bort-1]

Left out of draft as @ufechner7 set it, though the task is blocked: turned back 3 times in the last hour. The last of them: its session ended blocked.

[1-Bort-1]

Fixed in fe89ddf. Reproduced in Julia: 19 of 50 d_line values threw, as you replayed. The tether and the kite lines now get one material each, so nothing compares floats, and test/test-system-definition.jl sweeps those 50 diameters. The card now cites fe89ddf, gives KiteGeometry 0.3.2 allocations (no timings: box saturated, bench-end said discard), and with the changelog warns about Point next to GLMakie and GeometryBasics.