Skip to content

plot_slices_3d builds its own deflection filename tag, so it finds no - #299

Merged
1-Bart-1 merged 3 commits into
mainfrom
agent/298-plot-slices-3d-builds-its-own-deflection
Sep 12, 2026
Merged

plot_slices_3d builds its own deflection filename tag, so it finds no #299
1-Bart-1 merged 3 commits into
mainfrom
agent/298-plot-slices-3d-builds-its-own-deflection

Conversation

@1-Bort-1

@1-Bort-1 1-Bort-1 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

TL;DR

generated_slices built its own _d<degrees>.dat filename tag instead of calling AirfoilAero.delta_suffix, the function that named the file when generate_airfoils wrote it, so plot_slices_3d(dir; delta=...) silently drew no deflected contour for any negative or fractional deflection. It now asks delta_suffix for the name, leaving one source for it.

The red check is #300, not this diff

Test end-user and developer setup fails here on examples/ram_air_kite.jl, twice on the same commit, and it is not this change. That example asks for delta=1.0, and for a positive whole degree the old hand-rolled tag and delta_suffix(deg2rad(1.0)) produce the identical name _d1.dat — the two schemes diverge only on a minus sign or a decimal point. I evaluated both (_d1.dat from each) and then checked them against a generated directory built on this box from the example's own settings: same string, same file, all ten sections.

What actually fails is that the _d1.dat the XFoil sweep wrote on the runner is full of NaN NaN rows, so the strict read_dat_coordinates returns zero points and fit_kulfan_parameters throws on an empty argmin. write_section_aero writes every deflected column unconditionally, three lines above the guard that stops it writing an all-NaN zero-deflection file. Worse than the crash: read_section_aero uses the lenient read_dat, which parses NaN happily, so Wing(yaml) five lines earlier had already loaded a wing whose contour interpolants are NaN at every delta — including 0° — without a word. The crash on line 79 is the only thing that surfaced it. Filed as #300 with the mechanism and a ten-line reproduction.

I am deliberately not silencing it here by having generated_slices treat an unreadable .dat as missing. That would turn this check green while hiding a pipeline that quietly produces NaN wings, and it is a second idea in a diff that holds one. #299 should land after #300, or on the reviewer's judgement that the red check is pre-existing.

What was wrong

The two schemes agree only on positive whole degrees: delta_suffix writes a minus sign as m and a decimal point as p (_dm3.dat, _d2p5.dat), the local tag wrote them literally (_d-3.dat, _d2.5.dat). On the SK100's delta_range = -40:10:40 that is four of the eight non-zero deflections, and the warning the user got named the wrong scheme, so it pointed away from the real files.

Reproduced on the test suite's own generated ram-air directory (delta_range = -1:1:1), which writes airfoils/<i>_dm1.dat:

┌ Warning: No generated .dat for delta=-1.0° (4_d-1.dat, 3_d-1.dat, 2_d-1.dat, 1_d-1.dat); generated deflections are named airfoils/<i>_d<degrees>.dat.
└ @ VortexStepMethodMakieExt ext/VortexStepMethodMakieExt.jl:1642

What changed

One line replaces the two that rolled the tag: tag = "_$(AirfoilAero.delta_suffix(deg2rad(delta))).dat"generated_slices takes delta in degrees, delta_suffix in radians. That ends the cause rather than the case: there is now one function naming these files, so a later change to the scheme cannot desynchronise the reader from the writer again.

The warning is rewritten to drop the naming-scheme sentence, which was both wrong and redundant once the listed names are the ones actually looked for, and to name the directory instead — missing_dats now holds the path relative to out_dir rather than a bare basename, so the message says where it looked. The local was renamed from missing_deltas, which described deltas but held .dat paths.

The regression test reads the suite's cached ram_air_matrix_dir fixture through generated_slices at both signs of deflection. It reuses the exact (n_sections, alpha_range, delta_range) key the file's existing ram_air_matrix_wing call uses, so it shares that generation and adds no NeuralFoil sweep. Before the change the -1.0 iteration failed all three assertions (the warning fired, d2.def was empty, def3d was nothing) while +1.0 passed all three — 3 passed, 3 failed; after, 6 passed.

I searched for other hand-built deflection tags before writing (rg '"_d"', delta_suffix, dat_file): this was the only one. airfoils/<i>_raw.dat in geometry_gen.jl is the raw-points file and unrelated.

Where I would push back: the test covers the sign case (m), not the decimal case (p), because a fractional delta_range would key a second cached NeuralFoil generation for one extra character of coverage. Both come from the same single delta_suffix call, and write_section_aero's own round-trip test already exercises the function.

Verification

  • Reproduced first: the @warn above, against unchanged code
  • test/plotting/test_plotting.jl red before (3 passed, 3 failed), green after (6/6) — juliaserver, test env, include("plotting/test_plotting.jl")
  • Full plotting file green: Plotting (Makie) 58/58 · Airfoil skin (Makie) 19/19 · new testset 6/6
  • Up to date with origin/main (0 behind) · Julia lines ≤ 92 chars
  • Local CI mirror (agent ci-local, one matrix cell): PASS in 6 min
  • GitHub CI: Test end-user and developer setup FAILS, twice on a9fff16, for write_section_aero writes an all-NaN deflected .dat, and read_section_ #300 and not for this diff — evidence above. Every other check green.
  • jetls check: not run — no jetls binary on this box (bin/jetls wraps it)
  • n/a REUSE lint (repo carries no REUSE.toml/.reuse) · docs (no new or renamed public symbol; generated_slices is already listed in docs/src/private_functions.md)
  • Risk: the warning now interpolates the full out_dir path, so the message is longer than it was. Nothing asserts its text.

Scope

+26 / -9 across 3 files. ext/VortexStepMethodMakieExt.jl is the fix (net −2 lines, including one rewrapped docstring line in the same function); test/plotting/test_plotting.jl the regression test; CHANGELOG.md one Fixed entry. No stack — no open PR touches the extension. Closes #298. Blocked on #300 for a green board.

Closes #298 · task VortexStepMethod.jl-298

generated_slices built its own `_d<degrees>.dat` tag, which disagrees with
AirfoilAero.delta_suffix — the function that named the file — on every
negative or fractional deflection.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NsHXaNNeota4FBimt83s6S
@1-Bort-1 1-Bort-1 added agent:running Agent task state agent:ci Agent task state and removed agent:running Agent task state labels Sep 11, 2026
@1-Bort-1

1-Bort-1 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Local full suite: FAIL (1 min, Julia 1.12.7, one cell of the matrix)

    @ ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Test/src/Test.jl:1777 [inlined]
  [6] macro expansion
    @ ~/worktrees/VortexStepMethod.jl-298/test/runtests.jl:72 [inlined]
  [7] include(mapexpr::Function, mod::Module, _path::String)
    @ Base ./Base.jl:307
  [8] top-level scope
    @ none:6
  [9] eval(m::Module, e::Any)
    @ Core ./boot.jl:489
 [10] exec_options(opts::Base.JLOptions)
    @ Base ./client.jl:283
 [11] _start()
    @ Base ./client.jl:550
ERROR: Package VortexStepMethod errored during testing
Stacktrace:
  [1] pkgerror(msg::String)
    @ Pkg.Types ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/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.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/Operations.jl:2642
  [3] test
    @ ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/Operations.jl:2487 [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.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:552
  [5] test(pkgs::Vector{PackageSpec}; io::IOContext{IO}, kwargs::@Kwargs{})
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:169
  [6] test(pkgs::Vector{PackageSpec})
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:158
  [7] 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.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:186
  [8] test()
    @ Pkg.API ~/.julia/juliaup/julia-1.12.7+0.x64.linux.gnu/share/julia/stdlib/v1.12/Pkg/src/API.jl:175
  [9] top-level scope
    @ none:1
 [10] eval(m::Module, e::Any)
    @ Core ./boot.jl:489
 [11] exec_options(opts::Base.JLOptions)
    @ Base ./client.jl:283
 [12] _start()
    @ Base ./client.jl:550

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:ci Agent task state agent:queued Agent task state labels Sep 11, 2026
@1-Bort-1 1-Bort-1 added agent:ci Agent task state and removed agent:running Agent task state labels Sep 11, 2026
@1-Bort-1 1-Bort-1 added agent:waiting-human Agent task state and removed agent:ci Agent task state labels Sep 11, 2026
@1-Bart-1

Copy link
Copy Markdown
Member

Resolve the conflicts

@1-Bart-1
1-Bart-1 enabled auto-merge September 12, 2026 10:25
@1-Bort-1 1-Bort-1 added agent:queued Agent task state agent:running Agent task state and removed agent:waiting-human Agent task state agent:queued Agent task state labels Sep 12, 2026
…ection

Main's #312 rewrote the same block of `generated_slices` to skip a blank
deflected .dat. Kept its `skipped_deltas` structure and its blank-vs-missing
message, and put back the `delta_suffix` tag and the dropped naming-scheme
sentence; the skipped file is now named by its path under `out_dir`, which the
warning states. The changelog entry moved to a fresh `## Unreleased` section,
v5.1.0 having been cut in between.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017d2wohXpAyJ9qCV1FhoDJd
@1-Bart-1
1-Bart-1 merged commit 0d7097b into main Sep 12, 2026
9 checks passed
@1-Bort-1

Copy link
Copy Markdown
Contributor Author

Resolved in 60d71f0; #299 is mergeable and all nine checks are green, including Test end-user and developer setup, which #312 fixed. The three conflicts were main's #312 rewriting the same block of generated_slices; the resolution is at the top of the PR body.

@1-Bort-1 1-Bort-1 added agent:ci Agent task state and removed agent:running Agent task state labels Sep 12, 2026
@1-Bort-1 1-Bort-1 added agent:done Agent task state agent:queued Agent task state and removed agent:ci Agent task state agent:done Agent task state agent:queued Agent task state labels Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:done Agent task state

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plot_slices_3d builds its own deflection filename tag, so it finds no

2 participants