Fixing Constraints for new OM Parallelism Convention

bernardopacini commented 1 year ago

Purpose

The new OpenMDAO serial derivative parallelism convention (v3.25.0) made how we handled constraints invalid as we had different derivative values on different procs for serial inputs. The solution for this is to make constraints serial and consistent on each proc. I changed all the constraints in the MPhys wrapper as well as the compute() and compute_jacvec_product() functions to reflect these changes.

Expected time until merged

1 week.

Type of change

[X] 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
[X] Maintenance update
[ ] Other (please describe)

Testing

I tested this using the DAFoam MPhys aero-only and aerostructural tests. The derivatives are correct (even for TACS structural functionals) and OpenMDAO does not complain about derivative inconsistencies.

Checklist

[ ] I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
[ ] I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
[ ] 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] commented 1 year ago

Codecov Report

Merging #191 (e7a3452) into main (5b7066e) will increase coverage by 0.13%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #191      +/-   ##
==========================================
+ Coverage   64.75%   64.88%   +0.13%     
==========================================
  Files          47       47              
  Lines       11964    11939      -25     
==========================================
  Hits         7747     7747              
+ Misses       4217     4192      -25

Impacted Files	Coverage Δ
pygeo/mphys/mphys_dvgeo.py	`0.00% <0.00%> (ø)`

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

bernardopacini commented 1 year ago

@ArshSaja @hajdik Could you both please try running check_totals() on your MPhys cases with this branch and make sure you get correct derivatives and no warnings about mismatches on processors? You may need the latest OpenMDAO commit for this, not just the latest version.

joanibal commented 1 year ago

Sorry for not checking that the change worked. I appreciate you all fixing this. It's probably best not to wait on me for anything going forward.

mdolab / pygeo