Build a KiteGeometry SystemDefinition from KPS3 and KPS4 and carry it in the log

[1-Bort-1]

TL;DR

system_definition(s) describes a KPS3 or KPS4 model as a KiteGeometry SystemDefinition: its points, the springs it integrates as segments, one tether and one winch. topology_metadata(s) puts that definition into a saved log under the key topology. Until now a KPS4 run described its system only as a segment count, so nothing that reads a definition could draw it.

What it builds

• Points. One point per entry of s.pos, named "1" upwards as the schema asks of index-keyed writers. Point 1, the winch, is STATIC.
• Segments. For KPS4 the segments are s.springs as they stand: the 6 tether springs and the 9 bridle springs, with their p1/p2, rest length and axial_stiffness * length as unit_stiffness. Connectivity therefore comes from what loop! integrates, not from a second copy of SPRINGS_INPUT. The bridle segments have density 0, because KPS4 puts no mass on them, and keep d_line as the diameter they have in the drag term. KPS3 has no spring vector, so its segments join i and i + 1.
• Stiffness. unit_stiffness is the nominal value. stiffness_factor is left out on purpose: it is a start-up ramp that next_step! raises by 0.01 per step up to 1.0 (src/KiteModels.jl:749), and the torque-control example ends its run at 1.0. The test pins the nominal value.
• Extra mass. The KCU mass sits on the KCU point and the kite masses on points A–D. KPS3 puts kite and KCU mass on its last point. Tether mass is left to the segments' density and diameter.
• Tether and winch. The tether runs from point 1 to point segments + 1. The winch sits at point 1 and takes gear_ratio and drum_radius from the settings.
• Pose. The points are taken where they are when the definition is built. Unit mirror says pos_ENU is the initial pose, and that pose depends on init!'s arguments (delta, upwind_dir, steady-state solve). Building the definition right after init! is therefore the only way to get it, and the docstrings say so.

Where I'd push back

• The example is not literally unchanged. examples/reel_out_4p_torque_control.jl gains two lines: metadata = topology_metadata(kps4) after init!, and ; metadata on its save_log. Logger(particles, STEPS) never sees the model, and KiteUtils' Logger has no metadata slot, so an untouched script cannot carry the definition without global state. The simulation itself is unchanged. The other log-saving examples are left alone.
• connectivity_sha is written as "", and awesIO_version and schema as the literals "1.0.0" and "structure_schema.yml", through a positional Metadata(...). KiteGeometry's Metadata has no keyword constructor, no defaults and no exported schema version, and it has no SHA writer yet. Unit document adds the writer to structure_document. The defaults belong in KiteGeometry too, so that is where the literals should go. A third copy of the preimage rule here, beside SAM's and the coming KiteGeometry one, is the duplication the plan forbids.
• @reexport using KiteGeometry is what the thread asks for. It lets a KiteModels user build, read and compare the definition (SystemDefinition, Segment, structure_document) without a second using, as KiteModels already does for KitePodModels, WinchModels and AtmosphericModels. What it costs is Point, Segment, Tether, Winch and Metadata in every user's namespace. Aqua finds no ambiguity.
• The document is YAML text, not JSON. YAML is what structure_document round-trips today, and a JSON reader cannot parse it. to_json is also unit document's. Swapping the writer is then one line in topology_metadata.
• The thread says this repo writes aero_force_b and tether_induced_force per step. It writes neither. rg over src/, examples/ and test/ finds no hits. The sysstate rename has nothing to change here. What this unit needed from that side was KiteUtils 0.13's save_log(…; metadata), which Write KA orientations to SysState, keep KS inside the model #312 brings.
• Nothing to remove for mirror. KPS3 and KPS4 hold state in the model struct, as their integrators require, and no definition type mirrors it. The definition is built fresh from the model and carries no per-step fields.

Dependencies

• KiteGeometry is unregistered. It is pinned by [sources] to OpenSourceAWE/KiteGeometry.jl@1ca16d8, which is the merge of Generate SystemDefinition and its components from a recorded awesIO commit KiteGeometry.jl#26. [compat] is "0.1". The General registry rejects a package with a [sources] URL dependency, so while the pin stays, ./bin/release cannot register KiteModels. KiteGeometry has to be registered first, and then the [sources] entry comes out.
• YAML is added as a direct dependency, so the document can be written as a string. It was already in the manifest through KiteUtils.
• Manifests. Minimal resolve under 1.13 and under 1.12. Each .default moves by the one KiteGeometry entry and nothing else. The test project gains YAML to read the log back.

Verification

• Reproduced first: n/a, new feature. Red before the change: UndefVarError: system_definition not defined in Main.
• test/test-system-definition.jl red before, green after: 15/15 across 3 testsets (10 + 4 + 1). The KPS4 testset checks point count, segment endpoints against kps4.springs, positions, rest lengths, nominal unit_stiffness, tether, winch, total extra mass and massless bridle segments. The KPS3 testset covers its layout. A third testset checks that a saved and reloaded log parses back to the same structure document. Before the review round the bridle-density assertion failed against the previous code (8 passed, 2 failed), and it passes now.
• Acceptance: examples/reel_out_4p_torque_control.jl ran to the end and logged 1800 rows of 11 points. Its log's topology parses to n_points 11 (length(kps4.pos) 11) and 15 segments, whose endpoints equal kps4.springs (true). The metadata keys are created, frame_convention, kiteutils_version and topology.
• test/test-aqua.jl 9/9 (after the review round) · test/test-update-sys-state.jl and test/test-kps3.jl green.
• Local full suite (agent ci-local, Pkg.test(), Julia 1.13.0, fail-fast): FAIL, 337 passed, 0 failed, 1 errored, 11 broken. The one error is test/test-kps4.jl:581 test_find_steady_state ("solver returned non-finite values"). That is find_steady_state! does not converge for KPS4 on Julia 1.13, and CI hides it #314, which reports this error on main under 1.13, and it comes before test-system-definition.jl runs. The 11 broken are the ones already marked in test-kps4.jl. The box's later full-suite run failed at the same test, and test/test-kps4.jl alone in the session reproduces it with find_steady_state! does not converge for KPS4 on Julia 1.13, and CI hides it #314's signature: find_steady_state!: solver did not converge! (... iterations=247), then "non-finite values" at :581. The branch's only src changes are src/system_definition.jl and its include/reexport, and none of that is on find_steady_state!'s path. So this is neither a bug in the change nor in its test. find_steady_state! does not converge for KPS4 on Julia 1.13, and CI hides it #314 owns it, and nothing here was loosened.
• Docs build clean (after the review round) · REUSE lint clean on every tracked and new file (fsfe/reuse container) · stacked on Write KA orientations to SysState, keep KS inside the model #312's head 6bf76f9.
• GitHub CI on this draft runs only link check and REUSE lint (both pass); the test workflow does not run against the Write KA orientations to SysState, keep KS inside the model #312 base.
• Benchmark: n/a, not on the step path.
• Risk: the pos_ENU of a definition built after the run has started is the current pose, not the initial one. The docstrings say to build it after init!.

Scope

+190 / −5 across 10 files. src/system_definition.jl (73 lines) and its test (57 lines) are the change. The rest is the two-line example edit, the docs section (which also lists the internal assemble_system_definition), the changelog file, and 9 Project.toml lines across the root and test projects, plus 10 per .default for the KiteGeometry entry. Stack: on #312 (frame unification), which is on #321.

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: FAIL (1 min, Julia 1.13.0, one cell of the matrix)

    init_springs!           |   34                    34   0.0s
    init_masses!            |   11                    11   0.0s
    calc_particle_forces!   |   11                    11   0.2s
    init                    |   11                    11   0.0s
    initial_residual        |    1              1      2   0.3s
    inner_loop!             |   11                    11   0.1s
    calc_aero_forces!       |   11                    11   0.3s
    test_loop               |   33                    33   0.1s
    test_residual!          |   30              9     39   0.3s
    test_getters            |    7              1      8   0.0s
    test_find_steady_state  |           1              1   0.7s
RNG of the outermost testset: Random.Xoshiro(0x2f9a9109242dbe89, 0x3fc8ad88a372d75a, 0xbb261d06eeaeee36, 0xe12ca2d023347352, 0xbeeb9e76ae6012e3)
ERROR: Package KiteModels errored during testing
Stacktrace:
  [1] pkgerror(msg::String)
    @ Pkg.Types ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/Types.jl:68
  [2] test(ctx::Pkg.Types.Context, pkgs::Vector{PackageSpec}; coverage::Bool, julia_args::Cmd, test_args::Cmd, test_fn::Nothing, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool)
    @ Pkg.Operations ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/Operations.jl:3148
  [3] test
    @ ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/Operations.jl:3026 [inlined]
  [4] test(ctx::Pkg.Types.Context, pkgs::Vector{PackageSpec}; coverage::Bool, test_fn::Nothing, julia_args::Cmd, test_args::Cmd, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool, kwargs::@Kwargs{io::IOContext{IO}})
    @ Pkg.API ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/API.jl:586
  [5] kwcall(::@NamedTuple{io::IOContext{IO}}, ::typeof(Pkg.API.test), ctx::Pkg.Types.Context, pkgs::Vector{PackageSpec})
    @ Pkg.API ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/API.jl:562
  [6] test(pkgs::Vector{PackageSpec}; io::IOContext{IO}, kwargs::@Kwargs{})
    @ Pkg.API ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/API.jl:172
  [7] test(pkgs::Vector{PackageSpec})
    @ Pkg.API ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/API.jl:161
  [8] test(; name::Nothing, uuid::Nothing, version::Nothing, url::Nothing, rev::Nothing, path::Nothing, mode::PackageMode, subdir::Nothing, kwargs::@Kwargs{})
    @ Pkg.API ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/API.jl:189
  [9] test()
    @ Pkg.API ~/.julia/juliaup/julia-1.13.0+0.x64.linux.gnu/share/julia/stdlib/v1.13/Pkg/src/API.jl:178
 [10] top-level scope
    @ none:1
 [11] eval(m::Module, e::Any)
    @ Core ./boot.jl:489
 [12] exec_options(opts::Base.JLOptions)
    @ Base ./client.jl:310
 [13] _start()
    @ Base ./client.jl:577