Fix/xfoil sort empty arrays#176
Open
AhmedMoustafaa wants to merge 2 commits into
Open
Conversation
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.
Context
The XFoil 6.99 binary distributed via apt on Ubuntu is compiled with Fortran
FPE trapping enabled (
-ftrapuv -fpe0), causing it to crash with SIGFPEbefore producing any output. Users on Ubuntu who need a working XFoil must
build from source — typically XFoil 6.97, since 6.99 is no longer publicly
hosted. This PR fixes two parser bugs that surface when using a source-built
XFoil 6.97 on Ubuntu.
Fix 1 — 9-column polar output from source-built XFoil 6.97
XFoil 6.97 built from source with gfortran produces 9 data columns per row
against 8 header columns in the polar output file. This caused:
The exact cause is unclear — it may be related to how 6.97 handles the
cinccommand or a formatting difference between 6.97 and 6.99. The fixextends the existing monkey-patch block that already handles the analogous
XFoil 6.99 case (10 data columns against 8 header columns):
Fix 2 — IndexError when sorting output with unpopulated fields
After the column remapping in Fix 1,
Xcpminis absent from the 6.97 polarand remains as an empty array in the output dict after parsing. The sort step
at the end of
alpha()andcl()then raised:sort_orderhas length N (number of converged points) but indexing azero-length array raises immediately.
The fix drops unpopulated fields before sorting rather than making the sort
step handle mismatched lengths:
This is safe for callers:
CpminandXcpminwere never guaranteed to bepresent in the output — the docstring for
alpha()andcl()states onlythat "dictionary values are arrays; they may not be the same shape as your
input array if some points did not converge", with no promise of specific
keys. Code that needs these fields should already be using
.get().Note that Fix 2 is also a general defensive fix — any future code path that
leaves output arrays empty would previously have triggered the same IndexError.
Reproduction
Both bugs are reproducible on Ubuntu with XFoil 6.97 built from source using
gfortran. The apt XFoil 6.99 binary crashes with SIGFPE before reaching the
parser, so these bugs are only reachable after resolving the apt binary issue
(see linked issue)
Tests
Added
test_xfoil_polar_parsing.pywith two regression tests using a mockedpolar file matching XFoil 6.97 output format:
test_xfoil_697_cinc_column_mismatch— verifies the 9-column polar isparsed correctly and returns valid CL/CD values without raising XFoilError
test_xfoil_polar_sort_with_unpopulated_fields— verifiesalpha()returns correctly sorted results without IndexError when Xcpmin is absent
Test results
460 passed, same 2 pre-existing failures (
test_airplane_optimization,test_cadquery_export) present ondevelopbefore this PR.