Skip to content

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

Open
1-Bort-1 wants to merge 2 commits into
mainfrom
agent/298-plot-slices-3d-builds-its-own-deflection
Open

plot_slices_3d builds its own deflection filename tag, so it finds no #299
1-Bort-1 wants to merge 2 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

Copy link
Copy Markdown
Contributor Author

Local full suite: PASS (6 min, Julia 1.12.7, one cell of the matrix)

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ext/VortexStepMethodMakieExt.jl 75.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@1-Bort-1

Copy link
Copy Markdown
Contributor Author

CI: Test end-user and developer setup failed on a9fff16ca.

https://github.com/OpenSourceAWE/VortexStepMethod.jl/actions/runs/34585324837/job/103218084964

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:waiting-human 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