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

Multi-DVGeo capability in MPhys #207

Closed hajdik closed 1 year ago

hajdik commented 1 year ago

Purpose

This adds the ability to add multiple DVGeo objects to one MPhys geometry component when setting up the OM_DVGEOCOMP while preserving backward compatibility for the more common single DVGeo case. DVGeometry classes were also modified to standardize name and type attributes to enable tracking of multiple DVGeos.

Expected time until merged

1 week

Type of change

Testing

In addition to the MACH pyGeo tests passing, I have some test scripts for single-DVGeo and multi-DVGeo MPhys in place of actual MPhys tests.

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #207 (ff37ec2) into main (86e8e9b) will decrease coverage by 0.12%. The diff coverage is 7.03%.

@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
- Coverage   64.76%   64.65%   -0.12%     
==========================================
  Files          47       47              
  Lines       12018    12040      +22     
==========================================
  Hits         7784     7784              
- Misses       4234     4256      +22     
Files Changed Coverage Δ
pygeo/constraints/DVCon.py 71.87% <ø> (ø)
pygeo/constraints/areaConstraint.py 75.91% <0.00%> (ø)
pygeo/mphys/mphys_dvgeo.py 0.00% <0.00%> (ø)
pygeo/parameterization/BaseDVGeo.py 67.16% <100.00%> (+0.49%) :arrow_up:
pygeo/parameterization/DVGeo.py 68.76% <100.00%> (-0.02%) :arrow_down:
pygeo/parameterization/DVGeoCST.py 89.34% <100.00%> (ø)
pygeo/parameterization/DVGeoESP.py 66.57% <100.00%> (ø)
pygeo/parameterization/DVGeoSketch.py 67.92% <100.00%> (ø)
pygeo/parameterization/DVGeoVSP.py 81.97% <100.00%> (ø)

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

anilyil commented 1 year ago

The changes are good with me, but let's have the actual users review this PR. @bernardopacini @lamkina @ArshSaja

Maybe one simplifying pattern could be to always initialize the dvgeos as if you have multiple, but if dvgeo info is not passed, you can just populate that with the single dvgeo's information. Later on in the code, instead of looping over the keys, you can just take the first entry etc. But the way its currently coded is easier to understand; just more lines of code. Either way is fine with me.

hajdik commented 1 year ago

Maybe one simplifying pattern could be to always initialize the dvgeos as if you have multiple, but if dvgeo info is not passed, you can just populate that with the single dvgeo's information.

I implemented a version of this and tested it out, let me know what you think @anilyil

ArshSaja commented 1 year ago

My derivatives are fine. The only thing is that if I need to get some values from the DVGeo in my runscript, I would just use self.geo.DVGeo.getValues() earlier. But now I need to call it with the default DVGeo name: self.geo.DVGeos["defaultDVGeo"].getValues(). I think you should explain this in the MPhys wrapper.

hajdik commented 1 year ago

The only thing is that if I need to get some values from the DVGeo in my runscript, I would just use self.geo.DVGeo.getValues() earlier. But now I need to call it with the default DVGeo name: self.geo.DVGeos["defaultDVGeo"].getValues().

I think if you want to call a function that isn't in the MPhys wrapper like getValues(), the best way would be to use nom_getDVGeo() to get the DVGeo object and then call any functions directly on that.