mdolab / pygeo

pyGeo provides geometric design variables and constraints suitable for gradient-based optimization.
https://mdolab-pygeo.readthedocs-hosted.com/en/latest/?badge=latest
Apache License 2.0
122 stars 54 forks source link

Adding sliding curves for DVGeoMulti #231

Closed bernardopacini closed 7 months ago

bernardopacini commented 8 months ago

Purpose

There are cases when using the intersection method in which a curve on a surface should be preserved to make sure that the patch does not change size or the curve is maintained to a certain shape. For example, maintaining the interface line between a propeller hub and nacelle is important to ensure a realistic boundary condition on each patch. This PR adds this sort of tracking.

These changes are actually made by @anilyil.

Expected time until merged

2 weeks.

Type of change

Testing

I have run this locally and also checked the derivatives to make sure the code is correct.

Checklist

codecov[bot] commented 8 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (e47a454) 65.04% compared to head (61ca21f) 65.18%.

Files Patch % Lines
pygeo/parameterization/DVGeoMulti.py 95.45% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #231 +/- ## ========================================== + Coverage 65.04% 65.18% +0.13% ========================================== Files 47 47 Lines 12112 12123 +11 ========================================== + Hits 7878 7902 +24 + Misses 4234 4221 -13 ```

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

bernardopacini commented 8 months ago

I added a test for the sliding curve feature as discussed offline with @hajdik and @sseraj. It checks to ensure that a few points remain on the lines on which they were defined, but allowed to vary along them (@anilyil let me know if it is okay with you too).

bernardopacini commented 8 months ago

Thanks for the feedback @sseraj! I updated the files with your comments and defer to @anilyil on the implementation-specific comments.

anilyil commented 8 months ago

@sseraj, I addressed both of your comments. Also, thank you for spelling out pretty much all of the changes needed in the comments. Both things you said are correct. Please review my changes and mark the conversations as resolved.

@bernardopacini, this PR is good for another round of reviews, correct?

bernardopacini commented 8 months ago

@sseraj, I addressed both of your comments. Also, thank you for spelling out pretty much all of the changes needed in the comments. Both things you said are correct. Please review my changes and mark the conversations as resolved.

@bernardopacini, this PR is good for another round of reviews, correct?

I just updated the assertions as recommended by @sseraj, now should be good to go.