write_section_aero writes an all-NaN deflected .dat, and read_section_ - #301
Conversation
A deflection XFoil converged at no angle left generate_airfoil_aero's contour column at its NaN initial value; write_section_aero wrote it out as a .dat of `NaN NaN` rows, and read_section_aero's own tolerant reader parsed those back into the wing's contour interpolants, so every deflection interpolated to a NaN airfoil shape. write_section_aero now skips a deflection column with no finite contour, and the two .dat readers become one: the strict read_dat_coordinates moves into the main package beside the other loaders, drops NaN rows by value rather than by first character, and AirfoilAero imports it. read_section_aero warns and returns nothing when a contour does not hold its Cp table's nodes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013kHEhSNd6mzegjH4o8bjv9
|
Local full suite: PASS (12 min, Julia 1.12.7, one cell of the matrix) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…on-aero-writes-an-all-nan-def
…tes-an-all-nan-def' into agent/300-write-section-aero-writes-an-all-nan-def
|
Answering the question from the thread — what works, what does not, is deflected wrapping working, and did the continuous wrap regress anything. Measured on this branch; every number below is from a run I did today, not from a previous session. Did anything regress going continuous?Yes — two things, both found and fixed on the
The caveat I would not want buried: #293 (the double wrap) may not be independent of the rewrite. The mechanism that issue names — The second wrap multiplies the Kulfan weights by 5.1x on this small fixture while the shape still looks fine ( Is shrink wrap at delta != 0 working?Structurally yes, and I could not break it. Every deflection gives a finite, closed, 239-node contour — the node count is delta-independent, so the The deflection goes the right way, scales with the angle, and is symmetric in sign. What it is not is equal to a rigid rotation: the TE lands 12% short at small angles and 22–32% short at ±10°. Some of that is certainly my crude datum (mean The good news on the other half of #296: its So: what works, what does notWorks. The continuous wrap itself, at every delta, including zero clearance where it now hugs the input to 1e-9. Deflection geometry: finite, closed, delta-independent node count, right sign and right order of magnitude. Both solver backends at delta != 0 — NeuralFoil and XFoil agree within 0.03 on cl/cd/cm across the deflected grid ( Does not work, none of it the wrap's geometry:
One more thing the rewrite left behind, too small for its own task but I will not walk past it: What I would do next, in order
CI on this branchBoth red jobs earlier were pre-existing and are now fixed on main: the 1.12 coverage job was #287's |
|
Resolve the conflicts |
…-def Only CHANGELOG.md conflicted: v5.1.0 shipped and #312's audit-plot entry landed in the Fixed block this branch's entry sat in. The entry moves to Unreleased. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLm7oza8A7H8xtGQP2hp7Z
The merge was textually clean but semantically not: main's "generate_airfoils fits the wrapped contour it is handed" calls the test-local read_dat_coords this branch deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLm7oza8A7H8xtGQP2hp7Z
TL;DR
write_section_aerono longer writes a deflection whose contour is allNaN, and the two.datreaders that disagreed aboutNaNrows are now one strict reader. A deflection the 2D solver converged at no angle used to reach a loadedWingas aNaNairfoil shape at every deflection, 0° included, with no error and no warning.What was wrong
generate_airfoil_aerowrites a contour column only from a solution whose node count matches the grid's modal one, andXFoilSolverreturns empty node arrays for a non-converged angle — so a deflection that converged nowhere leavesx[:, jd]andy[:, jd]at theirfill(NaN, …)initial value.write_section_aerowrote that column out asairfoils/<id>_d<tag>.datfull ofNaN NaNrows, three lines above a guard whose comment already names the hazard for{dat_prefix}.dat.Then the readers disagreed.
read_dat_coordinates(submodule) kept only rows starting with a digit,-or., so an all-NaNfile read back as zero points and its caller crashed — that is the CI failure on #299,argminon an emptyxinsidenormalize_airfoil.read_dat(main package) had no such filter andtryparse(Float64, "NaN")succeeds, soread_section_aerogot a full-lengthNaNvector, the lengths matched, and theNaNs landed inx_interp/y_interp. Linear interpolation between a finite node and aNaNnode isNaNeverywhere, which is why the contour came outNaNat 0° as well. Ten lines reproduce it with no solver at all, aSectionAerowhose 1° column isNaN:What I changed
write_section_aerohoists thefinite(jd)helper it already had above the deflection loop and skips a column with no finite contour, so no all-NaN.datis ever written andisfilestays the honest test for a generated deflection — which is what the Makie audit plot already assumes, warning and drawing nothing for a deflection it finds no file for.The duplicate reader is gone rather than aligned.
read_dat_coordinatesmoves intosrc/section_aero.jlbesideread_node_tableandread_section_aero, since loading lives in the main package and writing inAirfoilAero;AirfoilAeroimports and re-exports it, so every existing caller and theusing VortexStepMethod.AirfoilAero: read_dat_coordinatesin the tests are unchanged. It now drops a row by value (isfinite) instead of by first character, which is what the first-character filter was approximating: that filter skippedNaN NaNonly becauseNis not a digit, and let0.5 NaNthrough. I searched for further copies before deleting the second one and found a third —read_dat_coordsintest/airfoil_aero/test_airfoil_aero.jl, a test-local reader of the same format — and deleted it too, in the file this PR already changes.read_section_aerois the last link: a deflected contour that is missing, or that does not hold exactly itsCptable's node count, now warns and returnsnothinginstead of quietly filling the interpolants.nothingis a state the loader already returns andadd_section!already takes, so an airfoil with a broken contour set loses its surface aero and keeps its polar, rather than poisoning the whole wing's geometry.The third item, and a better fix underneath it
Should
generate_airfoilsfail an id outright when a deflection converged at no angle, the wayall(isnan, clvals)already fails one for column 1? I would, and for the same reason: the coefficients of such a column survive only becausefill_node_nans!interpolated them from the neighbouring deflections, so what the id offers at that delta is a fabricated polar and, after this PR, no surface data at all.reuse_valid_airfoils=truethen maps those sections to the nearest id that solved everywhere — a visible, documented degradation — andreuse_valid_airfoils=falseerrors, which is what a caller who turned the fallback off asked to hear. The push-back is that it costs a whole airfoil for one fussy deflection, and #291 says that fixture's convergence moves between runs.There is a cheaper fix underneath it that I would rather see: the contour does not depend on the solver converging at all. It is
deform_section's own output, handed in, andNeuralFoilSolveralready carries it straight through (s.x = def.x). I measured what XFoil gives back atrepanel=false— 239 nodes againstdef's 239,max|dx| = max|dy| = 0.0at both 0° and 3° — sogenerate_airfoil_aerocould takex/yfromdefwhenever its node count matches the modal one and no geometry column would ever beNaN, for either backend. That does not subsume this PR: a converged deflection whose node count is not the modal one still leaves aNaNcolumn, so the writer's guard stays the backstop. Say the word and I will open it as its own task.This is the fix for the red
Test end-user and developer setupcheck that #299 carried, and that check is green here: with no all-NaN_d1.datwritten,generated_slicestakes itsisfilebranch and warns instead of crashing innormalize_airfoil. #295 names the same guard but is about a degenerate finite fit, and neither fix covers the other.The one red job is not this diff
Julia 1.12 - windows-latest - x64on5e78ab6is #287's flake, andmainsettles it rather than my say-so: twenty minutes after this run,main's own push CI for the #308 merge (run 34696079829) failed the identical assertion inJulia 1.12 - ubuntuatrelative_error(jac_fd, jac_fwd) = 0.0420681631196805, against this branch's0.04206403446037785attest/solver/test_forwarddiff.jl:87. Same test, same ~0.042 branch of the bimodal jump #287 records, on a commit carrying none of these seven files. Which cell fires moves run to run and OS to OS: windows red and 1.12 ubuntu green here, 1.12 ubuntu red and windows green onmain, and on the earlier3c4fd7eboth 1.12 ubuntu cells at 0.03999947 and 0.04001102 with windows green. It is a bug in the test — a 1e-4 bound over a discrete interpolant jump — and #287 owns it.So the failing check is not evidence about this diff, and I ruled the diff out by measurement rather than by argument.
test_forwarddiffbuilds its wing from a generated directory, which does go through the reader this PR rewrites, so I deleted that fixture and regenerated it through this branch's writer: 16.datfiles, not one with aNaNrow, and the strict reader returns coordinates identical to the lenient one it replaces on all 16. The change cannot alter what that test loads. On that fresh fixture the file is 7/7 green withnorm(jac_fwd) = 3.2971127281317214andrel_err = 5.22e-8— 1900x inside the bound, and 800000x below what CI saw.Merging
maininmainhas since taken #294, #299, #306, #310 and #312, and v5.1.0 went out. OnlyCHANGELOG.mdconflicted, both sides having added to the same### Fixedblock: my entry moves to## Unreleased, where it belongs now that v5.1.0 shipped without it, and both ofmain's entries stay verbatim.One conflict was semantic and the textual merge hid it. #294 added
@testset "generate_airfoils fits the wrapped contour it is handed"at the end oftest/airfoil_aero/test_airfoil_aero.jl, and it callsread_dat_coords— the test-local third reader this PR deletes. The merged file was clean togitandUndefVarErrorto Julia. Both calls now go throughread_dat_coordinates, which is the point of the deletion; the two files it reads are finite, so the strict reader returns the same coordinates.main's #312 guards the other end of the same bug — the audit plot skipping a deflected.datthat reads back empty — and the two do not overlap: #312 protects a directory already generated with an all-NaN.dat, this stops one being written. Its "Audit slices (Makie)" testset writes its blank file by hand, so it still exercises that guard with this writer in place, and passes 7/7 here.Verification
contour at delta=0.0: x=[NaN, NaN, NaN, NaN, NaN]from aSectionAerowhose only bad column is 1°, written and read back through the public writer and loadertest/airfoil_aero/test_airfoil_aero.jl— new testset "a deflection with no contour is neither written nor loaded as NaN" 2 passed / 3 failed before the change (the.datwas written, the load returned aSectionAero), 7/7 afterairfoil_aero/test_airfoil_aero.jl(87 over 11 testsets, Fit the contour generate_airfoils is handed, not a second wrap of it #294's new one among them at 2/2 and this PR's at 7/7),airfoil_aero/test_live_polar.jl(93),obj_adapter/test_obj_adapter.jl(52, the generate →Wing(yaml)round trip),solver/test_backend_comparison.jl(15),yaml_geometry/test_yaml_geometry.jl(178),plotting/test_plotting.jl(58 + 19 + Setup Test is failing #312's 6 and 7, the other caller of the moved reader),Aqua.jl(10) — no failuresdocs/make.jl, only the pre-existingsize_threshold_warnnotices onprivate_functions.mdandfunctions.md) · REUSE: n/a, nobin/reuse_lint· merged up toorigin/main(8b0c6bc)5e78ab6: six of seven checks green — Documentation,Julia 1.11 - ubuntu,Julia 1.12 - ubuntu,Julia 1.12 - macOS-aarch64, codecov/patch andTest end-user and developer setup.Julia 1.12 - windowsred on test_forwarddiff.jl's POLAR_MATRICES check flakes at ~4%, and the oper #287's flake alone (0.04206403446037785), with this PR's testset 7/7 inside it, andmainred on that same assertion at0.0420681631196805twenty minutes later. Local CI mirror (agent ci-local, one matrix cell) on the merged tree: PASS in 13mtest/solver/test_forwarddiff.jlon this branch, fixture deleted and regenerated through the new writer: 7/7 green,norm(jac_fwd) = 3.2971127281317214,rel_err = 5.22e-8against the 1e-4 bound (juliaserver, examples env)test/generated/fixture deleted and rebuilt through this branch's writer, then the two suites that consume them re-run, because the copies my earlier runs used were written before this branch took Fit the contour generate_airfoils is handed, not a second wrap of it #294:obj_adapter/test_obj_adapter.jl52/52 andplotting/test_plotting.jl58 + 19 + 6 + 7 — same counts as against the cached fixtures, so nothing in this PR's evidence rested on the stale contourread_section_aeroreturningnothingwhere it used to throw aSystemErrorfor a deflected contour it cannot open. Every generated directory on disk today with an all-NaN_d1.datnow warns and loads without surface aero instead of loading aNaNone, which is the intent — but a hand-made directory whose deflection tag does not matchdelta_suffixdegrades where it used to fail loudly.Scope
+70 / −65 across 7 files against the merge base (
8b0c6bc), of which 20 deleted lines are the two duplicate.datreaders and 23 are the moved one. No stack.Found on the way, not fixed here:
test/generated/'s fixture path is keyed onhash((n_sections, alpha_range, delta_range))and nothing about the code that fills it (test/test_data_utils.jl:46-49), so a box whose cache predates a contour-generation change keeps testing the old contour. It cost me a wrong number: my firsttest_forwarddiffrun on this merged tree reportednorm(jac_fwd) = 2.2059, #294's pre-fix doubly-wrapped value, from a directory generated the day before; regenerating gave3.29711. CI never sees it, checking out fresh each run. Keying the path on the generator's version, or stamping the config into the directory, is a change to shared test infrastructure that no test here needs — its own task, and worth one to whoever picks up #287, whose next step is reproducing that fixture.Also found, not fixed:
test/test_body_aerodynamics.jl,test/test_solver.jlandtest/test_yaml_geometry.jlare tracked and zero bytes — leftovers of the move intotest/<area>/, which nothing includes.includeing one silently does nothing, which is how I met them. Three file deletions in a diff that has no other reason to open them is acleanup:PR; say the word and I will open it.Closes #300 · task
VortexStepMethod.jl-300