Adding custom reference axis projections

[anilyil]

Purpose

The axis parameter when adding a reference axis determines how the links are created between each control point and the reference axis. The previous behavior only created rays from the control points and found the closest point between the ray and the reference axis to create the link from the control point to the reference axis.

This approach cannot guarantee that the links will lie on the same plane a section of the FFD cross section lies. To demonstrate this, consider a wing FFD that also has some dihedral built into it. The default axis direction is x so the code creates rays in the x direction from all control points. Then it finds the closest point from the ref axis to the ray. Due to the dihedral, the points on the upper surface will map slightly outboard, while the points on the lower surface will map inboard. See figure that demonstrates this default behavior:

In this PR, I add the capability to create the links based on the intersection of a plane and the reference axis. The plane for each control point is defined by the control point itself and a user supplied normal. The normal is handled through the axis parameter, and the code determines behavior based on the type of the input.

With the example above, if we set the axis parameter to np.array([0.0, 0.0, 1.0)], the code creates a plane for each control point with the normal pointing in the z direction. Then, the reference axis links are created using the intersection of these planes with the reference axis. As a result, the control point links lie in the same plane at each section as shown here:

I also added a test for this case. I add the ref axis to one of the example rectangular FFDs under the testing files with the axis direction [0, 1, 1]. The resulting links look like this:

The code then tests the vectors that define the links.

Expected time until merged

Less than a week hopefully. This is a fairly complete feature with an associated test.

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

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

Merging #168 (6917338) into main (b6e8676) will decrease coverage by 0.74%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
- Coverage   64.05%   63.30%   -0.75%     
==========================================
  Files          47       47              
  Lines       11814    11866      +52     
==========================================
- Hits         7567     7512      -55     
- Misses       4247     4354     +107     

Impacted Files	Coverage Δ
pygeo/pyNetwork.py	66.83% <84.61%> (+5.76%)	⬆️
pygeo/parameterization/DVGeo.py	66.34% <90.90%> (-0.02%)	⬇️
pygeo/constraints/areaConstraint.py	50.50% <0.00%> (-25.42%)	⬇️
pygeo/constraints/DVCon.py	68.48% <0.00%> (-3.25%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[sseraj]

I tried this out on my case because my wing has some dihedral. The method works as described. I left some comments on the implementation and documentation. Please also fix the black formatting.

One other thing is that the PR description says the normal for the wing FFD is in the y-direction. Shouldn't it be the z-direction?

[anilyil]

Thanks a lot for the review @sseraj. I addressed all of your comments, including the update in the PR description. Please let me know if there are anything else I need to fix in here.

[marcomangano]

Sorry for my late review. I am still going through the code, but at a first "deeper" look I think that the current axis option in addRefAxis is too confusing. The docstring is pretty clear but I feel that we should not have a kwarg that gives you two different behaviors depending on the input being a str or an array.
What if we split axis into rayaxis and normalaxis, where the first accepts the string and the second the numpy array? We can either throw an error if both options are passed, or decide to have the second overwriting the first one.

[anilyil]

That type of behavior change happens elsewhere in the code, and I added type checking explicitly for this. The problem with having multiple options that doe the same thing is then it is not clear at all which one takes precedence.

[marcomangano]

I understand the issue with multiple options. I still think this is not ideal but I am fine with it, and the docstring (+ the checks) is clear enough.
I only have an additional minor comment on a docstring.

Nice work, imho this is more intuitive than the default ray projection method.

[anilyil]

I will merge this once the tests pass. I want to see them run because we changed the pyspline code in the meanwhile. Thanks to @sseraj and @marcomangano for the reviews. And codecov passes for once how about that!