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
124 stars 55 forks source link

Added coordinate transformations and a few fixes #157

Closed anilyil closed 1 year ago

anilyil commented 1 year ago

Purpose

This PR adds the capability of doing coordinate transformations with pygeo. Specifically, we can now have the adflow geometry sit on a different reference frame but have a different reference frame for the parameterization. For example, the FFD can stay roughly aligned with the x-streamwise y-spanwise z-lift directions, whereas the CFD geometry is rotated and displaced. This is achieved by a callback function that the user provides that applies the transformations. Only differences in dvgeo is that we intercept the main add pointset, update, and derivative routines to apply the transformation as the first or the last thing that is done. Derivatives are also transferred with the same approach; we just include all of the rotations but dont apply the transformations. The same method can be implemented in other DVGeo classes as well.

Besides this, the PR has some minor bug fixes. Specifically, defining lower and upper bounds for sectional DVs were broken. This is fixed now. There is also a small fix for using pygeo with the symmetry option, but this may not be fully operational.

Expected time until merged

1-2 weeks

Type of change

Testing

Checklist

marcomangano commented 1 year ago

I know this is an early draft, but I am confused about what goes into self.coord_xfer. It looks like it has a very specific function signature. I am also not familiar with FFD configurations that would benefit from the symmetry option, but we can discuss that at a later stage.

anilyil commented 1 year ago

@marcomangano, you are correct that this is a draft, but I just added the explanation for that functionality: https://github.com/mdolab/pygeo/pull/157/files#diff-6f6a800de0054468f520f9d63ccbf34c246bc5cc490311587b7068ec8c27fb85R650-R708 Let me know if it's clear or not. I will also try to add a test for this.

anilyil commented 1 year ago

I am also thinking of adding the testing required for https://github.com/mdolab/pyspline/pull/47 in here. Is that okay? It looked like pyspline itself does not have many tests, plus, I would rather test the full embedding with DVGeo in the loop since thats how people interact with those routine 99% of the time.

codecov[bot] commented 1 year ago

Codecov Report

Merging #157 (7949637) into main (4641625) will decrease coverage by 10.52%. The diff coverage is 31.91%.

@@             Coverage Diff             @@
##             main     #157       +/-   ##
===========================================
- Coverage   63.75%   53.22%   -10.53%     
===========================================
  Files          47       47               
  Lines       11786    11819       +33     
===========================================
- Hits         7514     6291     -1223     
- Misses       4272     5528     +1256     
Impacted Files Coverage Δ
pygeo/pyBlock.py 45.37% <ø> (-1.65%) :arrow_down:
pygeo/parameterization/DVGeo.py 65.29% <30.43%> (-0.38%) :arrow_down:
pygeo/parameterization/designVars.py 77.65% <100.00%> (ø)
pygeo/parameterization/DVGeoMulti.py 0.39% <0.00%> (-89.38%) :arrow_down:
pygeo/constraints/areaConstraint.py 50.50% <0.00%> (-25.42%) :arrow_down:
pygeo/constraints/DVCon.py 68.04% <0.00%> (-3.69%) :arrow_down:
pygeo/topology.py 54.92% <0.00%> (-0.23%) :arrow_down:
pygeo/geo_utils/node_edge_face.py 45.71% <0.00%> (+2.44%) :arrow_up:
... and 2 more

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

anilyil commented 1 year ago

I added the tests from @sseraj's fuselage case. Some points are extremely challenging to converge, but I dont test everything. The set thats actively tested fails w/o the solver fixes in https://github.com/mdolab/pyspline/pull/47 and passes with these fixes.

The previous solver also had a termination criteria preventing the most problematic points from working. Now they do work, but you need a ridiculously large number of iterations. I only tested a few like this, but I am disabling them because that FFD is unreasonably skewed.

This is ready for review.

sseraj commented 1 year ago

To dos:

anilyil commented 1 year ago

All TODOs addressed. This one is ready for review.

anilyil commented 1 year ago

this is also ready for round 2. thanks again for all the feedback