Adding in child FFD functions

[bernardopacini]

Purpose

Adding a method for using parent + child FFDs in MPHYS. This is one possible implementation that I have been using with MPHYS + DAFoam and works as expected, but I think we should discuss if this is the best approach for how to do this. The tricky part is in the order of operations: child FFDs cannot be added to a parent without a reference axis, so they cannot be immediately set to the MPHYS geometry object. The configure method then goes in the wrong order, so we need an explicit call to nom_add_children() (added in this PR) to actually add the child FFDs after the reference axes are declared but before any dependent constraints are defined. The better solution here might require more careful modifications to pyGeo, but I am not sure what other issues we might run into if we were to do that.

EDIT: PR #146 broke the MPHYS wrapper that was using deprecated calls to add local and global design variables. I fixed that here, though that will require updating the MPHYS examples and tests on the MPHYS repo too.

Expected time until merged

This PR is open to discussion -- no specific timeframe set. 2 weeks.

Type of change

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (non-backwards-compatible fix or feature)
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Documentation update
• Maintenance update
• Other (please describe)

Testing

N / A

Checklist

• I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
• I have run unit and regression tests which pass locally with my changes
• I have added new tests that prove my fix is effective or that my feature works
• I have added necessary documentation

[bernardopacini]

Part of the formatting failure is with one of the regression tests (unrelated to this PR). I do not think there is an actual issue, so I am just going to ignore it with a #noqa: statement.

./tests/reg_tests/test_DVGeometryMulti.py:103:35: B023 Function definition does not bind loop variable 'nRefAxPts'.
                for i in range(1, nRefAxPts)

[codecov]

Codecov Report

Merging #144 (4ed8ec9) into main (7212c82) will decrease coverage by 10.54%.
The diff coverage is 3.84%.

@@             Coverage Diff             @@
##             main     #144       +/-   ##
===========================================
- Coverage   63.74%   53.19%   -10.55%     
===========================================
  Files          47       47               
  Lines       11689    11705       +16     
===========================================
- Hits         7451     6227     -1224     
- Misses       4238     5478     +1240     

Impacted Files	Coverage Δ
pygeo/mphys/mphys_dvgeo.py	0.00% <0.00%> (ø)
pygeo/parameterization/BaseDVGeo.py	70.00% <100.00%> (ø)
pygeo/parameterization/DVGeoMulti.py	0.40% <0.00%> (-89.42%)	⬇️
pygeo/constraints/areaConstraint.py	50.50% <0.00%> (-25.42%)	⬇️
pygeo/constraints/DVCon.py	68.04% <0.00%> (-3.69%)	⬇️
pygeo/pyBlock.py	45.37% <0.00%> (-1.65%)	⬇️
pygeo/parameterization/DVGeo.py	64.23% <0.00%> (-0.50%)	⬇️
pygeo/topology.py	54.92% <0.00%> (-0.23%)	⬇️
pygeo/__init__.py	91.30% <0.00%> (+8.69%)	⬆️
pygeo/parameterization/__init__.py	88.88% <0.00%> (+11.11%)	⬆️

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

[joanibal]

Looks good to me. I think we should look at pyGeo as discussed to see why the children FFD need reference axis before they are added.

[anilyil]

The code looks good. my only comment is that wouldn't it make more sense to use a dictionary approach for the child FFDs? That way we can index them by name instead of order of addition. We can use an ordered dictionary under the hood, or sort the names alphabetically and keep track of them that way. the users can pass dictionaries with child ffd name as the key and file as the value. Let me know if you think this is useful. I am also fine with merging this PR as is and doing that change separately

[bernardopacini]

The code looks good. my only comment is that wouldn't it make more sense to use a dictionary approach for the child FFDs? That way we can index them by name instead of order of addition. We can use an ordered dictionary under the hood, or sort the names alphabetically and keep track of them that way. the users can pass dictionaries with child ffd name as the key and file as the value. Let me know if you think this is useful. I am also fine with merging this PR as is and doing that change separately

That approach makes sense to me. I went with this approach because it matches how pyGeo handles children, but if the indexing is confusing we could add a dictionary. If @joanibal is on board too I'll convert it to using dictionaries instead.

Edit: The additional reason for doing this is to be able to use the childIdx= style used for constraints when defining design variables. Since we have to set the childIdx for constraints, a user would have to keep track of the number anyways. We could change these to all be with a dictionary, but we'd be deviating further from pyGeo.

[bernardopacini]

@joanibal and I found that with a small modification to pyGeo we can actually simplify adding child FFDs in MPHYS. Hold off on merging this PR until we finalize that.

[bernardopacini]

Now that PR #150 is merged, I will rework this to simplify it and push new changes. I'll change this to draft until it is ready.

[bernardopacini]

I fixed the package import in BaseDVGeo.py from:

from typing import OrderedDict

to

from collections import OrderedDict

as we discussed offline.

This is now good to go!