Added option to project thickness contraints

[joanibal]

Purpose

This PR adds the option to use the component of the toothpick thickness aligned with the original thickness direction for the constraint value. I added this feature to combat situations where the skew of my toothpick constraints was allowing for self intersection.

In the figure below the gray mesh is the deformed FFD, the red line is the original directions of the thickness constraints and the thick black lines are the thickness constraints. The original direction of the thickness constraint is purely in the y-axis, but it is skewd by a deformation of FFD in the x-axis. With the an unprojected thickness constraint this would result in an increased thickness value, but with this projected constraint it would not.

Expected time until merged

a week would be nice

Type of change

• [ ] Bugfix (non-breaking change which fixes an issue)
• [x] 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

See the new tests added to test_DVConstraints.py

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
• [x] I have run unit and regression tests which pass locally with my changes
• [x] I have added new tests that prove my fix is effective or that my feature works
• [ ] I have added necessary documentation

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 68.33333% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 65.46%. Comparing base (820ef71) to head (f60d9ad).

Files	Patch %	Lines
pygeo/constraints/thicknessConstraint.py	62.74%	19 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #238 +/- ## ======================================= Coverage 65.45% 65.46% ======================================= Files 47 47 Lines 12208 12265 +57 ======================================= + Hits 7991 8029 +38 - Misses 4217 4236 +19 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[joanibal]

This is ready for review

[marcomangano]

This looks really interesting, I will try and take a look at this in the next few days. Can I ask what kind of DV setup / optimization made you bump into this issue?

[joanibal]

Thank Marco. I'm doing some funky stuff with OpenVSP where I'm changing the shape in the x and y-axis.

[lamkina]

I also want to take a look at this PR this week and dig into this problem some more. I have been facing this issue with toothpicks and OpenVSP for a few years now.

[joanibal]

It's ready for review again

[lamkina]

I have a few concerns about this change, but it's mostly related to using thickness constraints with VSP generated surfaces in general.

VSP uses Bézier curves and surfaces to represent geometry. However, Bezier are not piecewise continuous like BSplines so the surface is actually decomposed into many Bézier patches. Each patch has its own u, v parameter space that can change individually. As we deform the surfaces in directions that are not parallel to the initial thickness constraints, the u, v parameters move on the individual surfaces and our initial mapping for toothpick endpoints is skewed.

With that said, if you change the top and bottom surface patches in directions non-parallel to the initial toothpick directions, it will actually move the entire constraint. This is very easy to do with VSP parameterizations. Again, this is a symptom of the thickness constraints in general with VSP, not just specific to this PR.

However, I do think this PR is a bit of a misnomer in the sense it's not actually fixing the underlying issue with the thickness constraints. In reality, the fix for this would be doing a nonlinear solve to find the new parametric coordinates that preserve the initial Cartesian orientation of the thickness constraints. I even think this issue permeates every geometry where we apply toothpick constraints with non-parallel deformations to the geometry. Most times this deformation is small and not picked up, but with VSP it is more noticeable because each Bezier patch has it's own parameter space.

[joanibal]

With that said, if you change the top and bottom surface patches in directions non-perpendicular to the initial toothpick directions, it will actually move the entire constraint. This is very easy to do with VSP parameterizations. Again, this is a symptom of the thickness constraints in general with VSP, not just specific to this PR.

I actually want the entire constraint to move for my particular case so I see this as a feature. I can see why you would also want constraints that are reprojected to the UV coordinate space.

However, I do think this PR is a bit of a misnomer in the sense it's not actually fixing the underlying issue with the thickness constraints.

I understand what you are saying and made the intentional choice to take a simpler path. If time wasn't a factor, it would be nice to have a constraint where one end is locked to a U,V coordinate and the other end is reprojected to UV space. For me this PR improves upon the issue of skewed thickness constraints enough to be worth adding to the code even if there hypothetically better means. In regard to the name, I'm not opposed to changing it if you have another idea. Although, I'd say this using the dot product here is just projecting in a different sense.

Let me know if I haven't interpreted what you are seeing as the underlying issue correctly (a picture may help me here).

[lamkina]

I actually want the entire constraint to move for my particular case so I see this as a feature. I can see why you would also want constraints that are reprojected to the UV coordinate space.

I can see the utility of the constraints moving if you are changing the geometry in multiple axis directions. I don't have an issue with this in general, it's more that we don't clarify the behavior anywhere and it works differently depending on which tools created the underlying geometry.

I understand what you are saying and made the intentional choice to take a simpler path. If time wasn't a factor, it would be nice to have a constraint where one end is locked to a U,V coordinate and the other end is reprojected to UV space. For me this PR improves upon the issue of skewed thickness constraints enough to be worth adding to the code even if there hypothetically better means. In regard to the name, I'm not opposed to changing it if you have another idea. Although, I'd say this using the dot product here is just projecting in a different sense.

In this case, I am okay with adding the projected constraints, but I think we should add a description of the behavior to the docstrings of the toothpick constraints. We can tackle adding the feature I described above in a different PR, I didn't expect you to implement that as it's going to be involved. I just wanted to make sure you knew why these things are sliding around and skewing.

[lamkina]

In regard to the name, I'm not opposed to changing it if you have another idea. Although, I'd say this using the dot product here is just projecting in a different sense.

The naming scheme makes sense. In the future I think we will have a few flavors of the thickness constraints with more consistent behavior.

[joanibal]

it works differently depending on which tools created the underlying geometry.

I'm not following this part. If the constraints was embedded in an DVGeo FFD object and there were changes along multiple axis it should work the same.

[lamkina]

I'm not following this part. If the constraints was embedded in an DVGeo FFD object and there were changes along multiple axis it should work the same.

My fault, I thought you were directly changing VSP variables and forgot you mentioned you are using an FFD, in which case this works differently (but also kind of makes my point on why this is confusing).