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

SVD composite DVs for VSP and ESP #170

Closed ArshSaja closed 1 year ago

ArshSaja commented 1 year ago

Purpose

Expected time until merged

This PR includes the @nwu63 composite DV concept of FFD cases. A similar formulation is implemented to use the SVD transformation for VSP and ESP design variables. A test for VSP is also included.

Type of change

Testing

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #170 (e3916c9) into main (4ac53e0) will decrease coverage by 0.36%. The diff coverage is 61.02%.

@@            Coverage Diff             @@
##             main     #170      +/-   ##
==========================================
- Coverage   64.14%   63.78%   -0.36%     
==========================================
  Files          47       47              
  Lines       11866    11954      +88     
==========================================
+ Hits         7611     7625      +14     
- Misses       4255     4329      +74     
Impacted Files Coverage Δ
pygeo/mphys/mphys_dvgeo.py 0.00% <0.00%> (ø)
pygeo/parameterization/BaseDVGeo.py 66.66% <61.53%> (-3.34%) :arrow_down:
pygeo/parameterization/DVGeoSketch.py 68.22% <64.17%> (-1.55%) :arrow_down:
pygeo/parameterization/DVGeoESP.py 66.29% <91.66%> (+1.41%) :arrow_up:
pygeo/parameterization/DVGeoVSP.py 81.54% <92.30%> (+0.11%) :arrow_up:
pygeo/parameterization/DVGeo.py 68.63% <100.00%> (+2.23%) :arrow_up:
pygeo/constraints/areaConstraint.py 50.50% <0.00%> (-25.42%) :arrow_down:
pygeo/constraints/DVCon.py 68.48% <0.00%> (-3.25%) :arrow_down:
pygeo/parameterization/designVars.py 79.25% <0.00%> (+1.59%) :arrow_up:
... and 1 more

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

ArshSaja commented 1 year ago

Test for FFD is added..

ArshSaja commented 1 year ago

What is the case in which you do not want to add input? Why is this only here for the VSP DV and none of the other types (from what I can tell)? @hajdik

When the composite DVs are added to openmdao, the default VSP DVs are DVs anymore. Therefore, we don't have output for these DVs. If the VSP DVs are added as inputs, OpenMDAO will consider them as DVs.

ArshSaja commented 1 year ago

Why was this argument added? It looks like s gets set to none in one case and returned from a function in the other case so the argument is always overwritten @hajdik

Right! the option was not necessary, it was removed.

hajdik commented 1 year ago

When the composite DVs are added to openmdao, the default VSP DVs are DVs anymore. Therefore, we don't have output for these DVs. If the VSP DVs are added as inputs, OpenMDAO will consider them as DVs.

So does this not happen for other composite DVs when used in MPhys?

ArshSaja commented 1 year ago

When the composite DVs are added to openmdao, the default VSP DVs are DVs anymore. Therefore, we don't have output for these DVs. If the VSP DVs are added as inputs, OpenMDAO will consider them as DVs.

So does this not happen for other composite DVs when used in MPhys?

This is the same for all composite DV cases. The optimizer has the information on composite DVs only, it doesn't have the information on the default DVs from VSP. pyGeo does the transformation from composite to default DVs when the necessary changes are required inside it.

hajdik commented 1 year ago

This is the same for all composite DV cases. The optimizer has the information on composite DVs only, it doesn't have the information on the default DVs from VSP. pyGeo does the transformation from composite to default DVs when the necessary changes are required inside it.

If it would be the same for all composite DVs, shouldn't the if add_input statement be added to all of the nom_add functions so that MPhys will work with all DVGeos and composite DVs? Also, could you add a comment explaining why the if statement is there to the code so it is more clear?

ArshSaja commented 1 year ago

This is the same for all composite DV cases. The optimizer has the information on composite DVs only, it doesn't have the information on the default DVs from VSP. pyGeo does the transformation from composite to default DVs when the necessary changes are required inside it.

If it would be the same for all composite DVs, shouldn't the if add_input statement be added to all of the nom_add functions so that MPhys will work with all DVGeos and composite DVs? Also, could you add a comment explaining why the if statement is there to the code so it is more clear?

Added the composite optionality to all DVGeo.

ArshSaja commented 1 year ago

The add_input is not required when we use composite DVs. so if isComposite is false by default, the input has to be added to the OpenMDAO. If isComposite is true, then we don't need to add the inputs. Now the name convention should be fine.

ewu63 commented 1 year ago

Have we verified that the FFD DVGeo SVD tests also pass on the main branch before the PR?

ArshSaja commented 1 year ago

Yes I tested the same test with main branch. It was working.