MPhys wrapper and boundary condition fail flag fixes

[lamkina]

Purpose

This PR fixes multiple bugs with properly passing the meshFamilyGroup from the MPhys wrapper to pyADflow function calls. Additionaly, this PR adds a way to add a custom design surface family called my_surf using the options in the ADflow Builder. Previously, there was no way to add a custom family group before the mesh is set in the ADflow Builder's initialize method.

Now, the user can specify a custom set of design surfaces as a family group and add them to the meshFamilyGroup.

This PR will also adress issue #148 where the totalSubsonicInlet functions was hanging on some procs when the normal vector was flipped for some cells along the boundary.

Finally, this PR changes the way family groups are stored in the forward mode matrix free jacobian vector products. Previously, this function uses sets which received a different ordering on each proc. Now, we will use lists with if/else logic that mimics a set. This will keep the order of family groups the same on all procs while maintaining "set like" behavior with no duplicate entries.

Expected time until merged

~~This PR should take a week or less.~~ This PR will take much longer than I originally thought.

Type of change

• [x] Bugfix (non-breaking change which fixes an issue)
• [x] 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

I tested this fix by wrapping the simple actuator zone test test_actuator.py with MPhys and adding a custom surface family. I included the FFD file, cgns mesh, and run script in the zip archive below. mphys_fix.zip

Checklist

• [x] I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
• [x] 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

[codecov[bot]]

Codecov Report

Merging #224 (147a53a) into main (ee39337) will decrease coverage by 0.05%. The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main     #224      +/-   ##
==========================================
- Coverage   41.35%   41.30%   -0.06%     
==========================================
  Files          13       13              
  Lines        3794     3806      +12     
==========================================
+ Hits         1569     1572       +3     
- Misses       2225     2234       +9     

Impacted Files	Coverage Δ
adflow/mphys/mphys_adflow.py	0.00% <0.00%> (ø)
adflow/pyADflow.py	68.43% <80.00%> (+0.01%)	:arrow_up:

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

[lamkina]

I fixed a bug that involves error checking for inflow boundary conditions. Previously, if there was a mesh failure near an inflow boundary condition that caused the normal vector on the bc face to flip direction, the proc that contained that bc cell would hang due to an mpi barrier call. With this fix, the flipped normal vector will throw a routineFailed error that we catch in the python layer after we update and check the bc's.

I plan to add testing for this error case as well as the mphys wrapper updates in a future PR.

[anilyil]

Also, I think other BC routines also have similar issues, its not just the one you fixed. In fact, the others just terminate the code instead of hanging. E.g. https://github.com/mdolab/adflow/blob/main/src/bcdata/BCData.F90#L969 Can we apply the same fix to the others? Are you sure the code does not hang and the processors end up with the correct fail flags when you do the allreduce in python?

[lamkina]

I looked over BCData and I think the only spot where we might want to add a returnFail is here: https://github.com/mdolab/adflow/blob/bbfaa706629005da6a44d1bcd6bdacd0a5efb31b/src/bcdata/BCData.F90#L1119. It seems that for a supersonic inflow BC we just terminate if the velocity ends up pointing in the wrong direction, however that seems like a place to throw a fail flag. @anilyil per your previous comment, does the terminate call at this line: https://github.com/mdolab/adflow/blob/main/src/bcdata/BCData.F90#L969 require us to throw a fail flag? I would think if one of the BC variables is not specified we want to terminate instead of throw a fail flag.

[lamkina]

Just a few minor detail comments. One general comment: Have you tried the MPhys examples with this change? Is the behavior the same when we dont specify a custom surface?

Just ran the ADflow mphys aero only examples and they work without custom surface families using this fix. Basically, if this option isn't set in the builder, then the parameter defaults to None and there's logic in the ADflowBuilder initialize method that checks if the parameter is None to determine if it should add custom families or not.

[lamkina]

Actually, it looks like you did not rerun tapenade after changing BCdata. Can you fix the issue? See here

Doing this now and re-running tests. Should be ready soon.