Efficient interior-only embedding

[sseraj]

Purpose

I was running into long startup times when using child FFDs. The problem is that the embedding process for child FFDs can involve many points outside of the child FFD volume(s), and projecting these exterior points is expensive. In addition, projecting exterior points is more expensive with pySpline v1.5.2 than v1.5.1 because of the change in convergence criteria made in mdolab/pyspline#47.

To reduce the cost, I added a convex hull check for child FFDs (interiorOnly=True). One of the properties of B-splines is that they are contained within the convex hull of its control points. This means that if a point is outside the convex hull of the child FFD control points, we can skip the projection step because the point cannot be inside the FFD volume(s).

To test this, I timed the projection of a wing triangulated surface to a parent wing FFD and two child flap FFDs. The times in seconds are given in the table below:

		Wing	LE flap	TE flap
pySpline v1.5.2	Convex hull check	8.5	1.6	0.7
	Bounding box check	8.5	19.4	1.7
	No check	8.5	125.3	49.1
pySpline v1.5.1	Convex hull check	9.0	2.1	0.8
	Bounding box check	8.9	5.3	0.9
	No check	9.1	24.6	15.4

The projection times for the child FFDs are much faster with the convex hull check. The cost is similar now between pySpline versions because most exterior points are never passed to the projection routine.

I also initially tried a bounding box check, which you can see in the first commit on this branch. The bounding box is larger than the convex hull, so the projection is more expensive if the bounding box is not a close match for the FFD volume(s).

Expected time until merged

1 week

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)
• [x] Refactoring (no functional changes, no API changes)
• [ ] Documentation update
• [ ] Maintenance update
• [ ] Other (please describe)

Testing

One-level FFD behavior is unchanged. The current child FFD tests pass, including the box constraint tests, which test the edge case where the FFD volume and convex hull are coincident with points lying on surface of the FFD volume. I also tested this on more realistic wing and full configuration cases.

Checklist

• [x] 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
• [x] 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
• [x] I have added necessary documentation

[codecov[bot]]

Codecov Report

Merging #179 (3634dc3) into main (1f6a7c3) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #179      +/-   ##
==========================================
+ Coverage   64.74%   64.76%   +0.02%     
==========================================
  Files          47       47              
  Lines       11941    11949       +8     
==========================================
+ Hits         7731     7739       +8     
  Misses       4210     4210              

Impacted Files	Coverage Δ
pygeo/pyBlock.py	47.87% <100.00%> (+0.85%)	:arrow_up:

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

[anilyil]

I suggest we combine the changes from #181 in this PR. I want the embedding logic to work with a few different scenarios: The points can be outside the convex hull, the points can be between convex hull and the embedding volume, points can be inside embedding volume but projection doesn't converge, and finally, points can be inside the volume and projection works.

On top of this, the user may want all points to be tracked by the FFD, or only the points inside the embedding volume to be tracked.

Right now, the parent FFD will try to embed all points; if some are outside the embedding volume, the closest projection is picked. We address this in #181 by throwing an error because this modifies the surface geometry without the users realizing. This default behavior was originally to catch any projection tolerance issues when points were inside the domain, but with the recent changes, these should happen much less frequently.

I suggest we combine the convex hull check with these checks. There is also a use case where users only want part of the geometry modified. Previously, people would recommend using a parent FFD that includes the entire geometry while having a child FFD for the local design changes. This is absolutely pointless and we can just get the same effect by letting the FFD only work with the embedded points. This behavior can be achieved right now, but it is not intuitive; the users would need to pass an additional kwargs to the setDVGeo call.

My question for @sseraj, @ArshSaja, and @hajdik: Do we want to handle these logical checks in this PR, or merge this, and work on the logical checks separately? I also want to add a more intuitive way to enable the "local FFD" approach where the entire geometry is not contained within the FFD, but the FFD only works with the points that can be successfully embedded.

For example, it would be great if we could have a mode where the embedding just ignores all points outside the convex hull, and raises either a warning or an error if points are inside the convex hull, but cannot be fully embedded, which means they are either outside the embedding volume or the solver did not work. This would raise an error for the most general FFD usage case where the entire geometry is inside the FFD, and only a warning when a "local FFD" is used. Similarly, we can raise warnings when child FFDs are used but some points inside their convex hull is not embedded properly. Right now, there is no feedback for this, and users only realize embedding errors after they change the design on the child FFD.

[hajdik]

Do we want to handle these logical checks in this PR, or merge this, and work on the logical checks separately?

I'm fine either way. We could merge this now if there is anyone who needs this speedup on child FFDs right now, or keep all the changes together if we want one cohesive PR on embedding improvements.

[anilyil]

I would slightly prefer having the changes contained in one PR, but it is up to @sseraj.

[anilyil]

@sseraj suggested merging this PR as is because it does not change the default behavior and only affects performance. So we will do my suggested changes in #181