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

CST airfoil parameterization #141

Closed eytanadler closed 2 years ago

eytanadler commented 2 years ago

Purpose

This PR adds a CST parameterization for airfoil optimization. It allows for design variables of the CST parameters, class shape, and chord length. All derivatives analytically defined.

Expected time until merged

2 furlongs

Type of change

Testing

Tests for the new DVGeometryCST can be found in tests/reg_tests/test_DVGeometryCST.py.

Checklist

codecov[bot] commented 2 years ago

Codecov Report

Merging #141 (01b5c47) into main (c417b0f) will decrease coverage by 9.54%. The diff coverage is 89.37%.

@@            Coverage Diff             @@
##             main     #141      +/-   ##
==========================================
- Coverage   62.78%   53.24%   -9.55%     
==========================================
  Files          46       47       +1     
  Lines       11266    11679     +413     
==========================================
- Hits         7073     6218     -855     
- Misses       4193     5461    +1268     
Impacted Files Coverage Δ
pygeo/parameterization/DVGeoCST.py 89.18% <89.18%> (ø)
pygeo/__init__.py 100.00% <100.00%> (+10.52%) :arrow_up:
pygeo/parameterization/__init__.py 100.00% <100.00%> (+14.28%) :arrow_up:
pygeo/parameterization/designVars.py 77.65% <100.00%> (+0.48%) :arrow_up:
pygeo/parameterization/DVGeoMulti.py 0.40% <0.00%> (-89.42%) :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/pyBlock.py 45.37% <0.00%> (-1.65%) :arrow_down:
pygeo/parameterization/DVGeo.py 63.96% <0.00%> (-0.50%) :arrow_down:
pygeo/topology.py 54.92% <0.00%> (-0.23%) :arrow_down:
... and 2 more

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

ewu63 commented 2 years ago

Do we want to find another reviewer to expedite the process? Maybe @joanibal?

eytanadler commented 2 years ago

Not sure what's going on with flake8, but it must be due to a new version since it's from code I didn't touch. What's the best approach to handle that?

ewu63 commented 2 years ago

Not sure what's going on with flake8, but it must be due to a new version since it's from code I didn't touch. What's the best approach to handle that?

Yeah this is from flake8-bugbear which just had a new version released. It gets a bit tricky pinning versions of all packages but I guess we could do it. Though for this kind of thing new versions bring better bug detection.

eytanadler commented 2 years ago

Not sure what's going on with flake8, but it must be due to a new version since it's from code I didn't touch. What's the best approach to handle that?

Yeah this is from flake8-bugbear which just had a new version released. It gets a bit tricky pinning versions of all packages but I guess we could do it. Though for this kind of thing new versions bring better bug detection.

Got it. I'm happy to fix it in this PR, I'm just not exactly sure what it needs. I tried adding a nonlocal, but that didn't do it.

ewu63 commented 2 years ago

Yeah at first glance I'm not really sure either. If we can't figure it out we can always tell flake8 to skip the warning.

anilyil commented 2 years ago

It looks like the flake check is complaining that the twist function defined in the dvgeomulti test can vary with different axes. Especially, the nrefaxis variable there might be getting updated, or there's a risk of it. we can just add it as an optional parameter to the function and set the value when we define the function. maybe that way it wont complain. sth like:

            # Set up a twist variable
            def twist(val, geo, nRefAxPts=nRefAxPts):
                for i in range(1, nRefAxPts):
                    geo.rot_z["box"].coef[i] = val[i - 1]

I don't know if this will fix it though.

eytanadler commented 2 years ago

It looks like the flake check is complaining that the twist function defined in the dvgeomulti test can vary with different axes. Especially, the nrefaxis variable there might be getting updated, or there's a risk of it. we can just add it as an optional parameter to the function and set the value when we define the function. maybe that way it wont complain. sth like:

            # Set up a twist variable
            def twist(val, geo, nRefAxPts=nRefAxPts):
                for i in range(1, nRefAxPts):
                    geo.rot_z["box"].coef[i] = val[i - 1]

I don't know if this will fix it though.

Nice! Looks like this did the trick