CST leading and trailing edge fixes and improvements

[sseraj]

Purpose

I made a few fixes and improvements to how the CST implementation handles leading and trailing edges:

1. Fixed the shape function coefficient fitting by subtracting the linear trailing edge thickness function from the y-coordinates before the least-squares solution
2. Tracked the upper and lower surface trailing edge y-coordinates separately. Some CST implementations only use one thickness variable, but tracking the upper and lower surfaces separately is more general and is consistent with the implementation in ESP.
3. Split the upper and lower surfaces for the CST fit based on LE and TE location instead of the spline projection method in _splitUpperLower. _splitUpperLower associates each point with either the upper or lower surface. This makes sense for addPointSet calls but not for the CST fit. When the airfoil has a single LE point or a sharp TE, those points should be included in both the upper and lower surfaces. This change ensures that symmetric airfoils have symmetric CST coefficients.
4. Fixed the chord normalization for calls to computeCSTCoordinates in the fit error check and in test_fitCST
5. Added trailing edge thickness arguments to plotCST

Expected time until merged

Not urgent, 2-3 weeks would be nice

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (non-backwards-compatible fix or feature)
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Documentation update
• Maintenance update
• Other (please describe)

Testing

To verify the code changes, I:

1. Changed the existing trailing edge thickness tests to check the upper and lower surface y-coordinates
2. Added tests that check that coefficients for symmetric airfoils are symmetric (including symmetric airfoils with one LE point)
3. Verified the CST fit against the Kulfan class in pyESP and added regression tests for the coefficients (the reference is still DVGeometryCST)

Here is an example of how the CST coefficients change with this PR (for a NACA 0012 airfoil with leading edge at 0,0). The fit error computed by the internal check is lower, the upper and lower surface coefficients are symmetric, and the coefficients match pyESP.

naca0012_zeroLE.dat on main branch

Upper surface
    L2 norm of coordinates in dat file versus fit coordinates: 0.0006331960041569371
    Fit CST coefficients: [0.17289704 0.15522273 0.16084008 0.1639404  0.10807389 0.19881113
 0.08805734 0.19614525]
Lower surface
    L2 norm of coordinates in dat file versus fit coordinates: 0.0006288295041572267
    Fit CST coefficients: [-0.17253255 -0.15627056 -0.1586615  -0.16706265 -0.1049002  -0.20109143
 -0.08692161 -0.19651448]

naca0012_zeroLE.dat on PR branch

Upper surface
    L2 norm of coordinates in dat file versus fit coordinates: 3.476089168663822e-05
    Fit CST coefficients: [0.17309584 0.15072331 0.17568197 0.12476168 0.1680704  0.1238723
 0.14725536 0.13902121]
Lower surface
    L2 norm of coordinates in dat file versus fit coordinates: 3.4760891686639015e-05
    Fit CST coefficients: [-0.17309584 -0.15072331 -0.17568197 -0.12476168 -0.1680704  -0.1238723
 -0.14725536 -0.13902121]

naca0012_zeroLE.dat with pyESP

Upper surface coefficients [0.17309583808521334 0.15072330799759134 0.17568197194146518 0.12476168056503996 
0.1680704011542561 0.12387230726234683 0.14725535540361023 0.1390212120520205] dimensionless

Lower surface coefficients [-0.17309583807638124 -0.1507233079536379 -0.17568197237881203 -0.12476167919329707 
-0.16807040351349162 -0.12387230471899825 -0.14725535719212318 -0.13902121124037606] dimensionless

Checklist

• I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
• I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
• I have run unit and regression tests which pass locally with my changes
• I have added new tests that prove my fix is effective or that my feature works
• I have added necessary documentation

[codecov]

Codecov Report

Attention: Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 65.42%. Comparing base (3471013) to head (0a099fa).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
pygeo/parameterization/DVGeoCST.py	95.91%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #252      +/-   ##
==========================================
+ Coverage   65.35%   65.42%   +0.06%     
==========================================
  Files          47       47              
  Lines       12286    12307      +21     
==========================================
+ Hits         8030     8052      +22     
+ Misses       4256     4255       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[eytanadler]

These look like very useful updates, thanks Sabet! I took a first look and have a few comments/questions.

Also since the interface changes, does this merit a version update?

[eytanadler]

Does this check break if the airfoil is sharp? Might be better to check indices 1 and -2 if so.

[sseraj]

This actually works for sharp airfoils because of the check above it that removes duplicate points

[eytanadler]

Many airfoils (e.g., the RAE 2822) have a lower surface slope at the TE that makes the second-to-last point on the lower surface actually has a positive y value. So it might think the lower surface is the upper surface. Would that break this check?

I'm trying to think of all the corner cases and come up with a potentially more robust method. Checking the second to last would work if the x coordinates of all the upper/lower points are the same, but maybe there's some edge case where given different upper and lower x coordinates the y values will make this check wrong. Maybe there's some way to take a vector between two points on each surface at the TE and do some geometry with it? You might be able to use the sign of the Z component of the cross product between the vectors at the TE (idx 1 - 0 and idx -1 - -2) as a check. I haven't thought it through fully, but there's gotta be something simple and robust.

[sseraj]

Those are both cases where the current check could break. The cross-product approach should work

[eytanadler]

Is this ready to go and/or be reviewed by others?

[sseraj]

Is this ready to go and/or be reviewed by others?

Yes, this is ready for both

[marcomangano]

Sorry for the delay, I was not familiar with this part of the codebase so I had to learn it on the fly. Great job, especially with the updated tests!!

[sseraj]

Thanks for the reviews!