plot_slices_3d builds its own deflection filename tag, so it finds no - #299
Open
1-Bort-1 wants to merge 2 commits into
Open
plot_slices_3d builds its own deflection filename tag, so it finds no #2991-Bort-1 wants to merge 2 commits into
1-Bort-1 wants to merge 2 commits into
Conversation
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
Contributor
Author
|
Local full suite: PASS (6 min, Julia 1.12.7, one cell of the matrix) |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Author
|
CI: https://github.com/OpenSourceAWE/VortexStepMethod.jl/actions/runs/34585324837/job/103218084964 |
6 tasks
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
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.
TL;DR
generated_slicesbuilt its own_d<degrees>.datfilename tag instead of callingAirfoilAero.delta_suffix, the function that named the file whengenerate_airfoilswrote it, soplot_slices_3d(dir; delta=...)silently drew no deflected contour for any negative or fractional deflection. It now asksdelta_suffixfor the name, leaving one source for it.The red check is #300, not this diff
Test end-user and developer setupfails here onexamples/ram_air_kite.jl, twice on the same commit, and it is not this change. That example asks fordelta=1.0, and for a positive whole degree the old hand-rolled tag anddelta_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.datfrom 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.datthe XFoil sweep wrote on the runner is full ofNaN NaNrows, so the strictread_dat_coordinatesreturns zero points andfit_kulfan_parametersthrows on an emptyargmin.write_section_aerowrites 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_aerouses the lenientread_dat, which parsesNaNhappily, soWing(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_slicestreat an unreadable.datas 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_suffixwrites a minus sign asmand a decimal point asp(_dm3.dat,_d2p5.dat), the local tag wrote them literally (_d-3.dat,_d2.5.dat). On the SK100'sdelta_range = -40:10:40that 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 writesairfoils/<i>_dm1.dat:What changed
One line replaces the two that rolled the tag:
tag = "_$(AirfoilAero.delta_suffix(deg2rad(delta))).dat"—generated_slicestakesdeltain degrees,delta_suffixin 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_datsnow holds the path relative toout_dirrather than a bare basename, so the message says where it looked. The local was renamed frommissing_deltas, which described deltas but held.datpaths.The regression test reads the suite's cached
ram_air_matrix_dirfixture throughgenerated_slicesat both signs of deflection. It reuses the exact(n_sections, alpha_range, delta_range)key the file's existingram_air_matrix_wingcall uses, so it shares that generation and adds no NeuralFoil sweep. Before the change the-1.0iteration failed all three assertions (the warning fired,d2.defwas empty,def3dwasnothing) while+1.0passed 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.datingeometry_gen.jlis 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 fractionaldelta_rangewould key a second cached NeuralFoil generation for one extra character of coverage. Both come from the same singledelta_suffixcall, andwrite_section_aero's own round-trip test already exercises the function.Verification
@warnabove, against unchanged codetest/plotting/test_plotting.jlred before (3 passed, 3 failed), green after (6/6) — juliaserver, test env,include("plotting/test_plotting.jl")Plotting (Makie)58/58 ·Airfoil skin (Makie)19/19 · new testset 6/6origin/main(0 behind) · Julia lines ≤ 92 charsagent ci-local, one matrix cell): PASS in 6 minTest end-user and developer setupFAILS, twice ona9fff16, 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 — nojetlsbinary on this box (bin/jetlswraps it)REUSE.toml/.reuse) · docs (no new or renamed public symbol;generated_slicesis already listed indocs/src/private_functions.md)out_dirpath, so the message is longer than it was. Nothing asserts its text.Scope
+26 / -9 across 3 files.
ext/VortexStepMethodMakieExt.jlis the fix (net −2 lines, including one rewrapped docstring line in the same function);test/plotting/test_plotting.jlthe regression test;CHANGELOG.mdoneFixedentry. No stack — no open PR touches the extension. Closes #298. Blocked on #300 for a green board.Closes #298 · task
VortexStepMethod.jl-298