Fix bug breaking multiple DVGeos for MPhys

hajdik commented 5 months ago

Purpose

A small change introduced to DVGeo with how DV names were handled wasn't propagated to the MPhys wrapper. This change was to append the DVGeo name to the DV name when a name is present, which mostly only happens when you have multiple DVGeos to keep track of. MPhys wasn't expecting this full name so it wasn't accumulating the derivatives and they were all 0. Try this one cool trick to get 0-1s in one iteration!

I also cleaned up some random duplicate code and simplified the handling of child DVGeos in MPhys

Expected time until merged

This isn't urgent, likely no one else has this use case

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

Testing

Creating two DVGeo objects (or just naming one) leads to immediate 0-1s. Maybe someone with MPhys scripts (especially using child DVGeos) should run using this to make sure I didn't miss anything?

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

** is it time to talk about how we can implement tests for our MPhys wrappers

hajdik commented 5 months ago

@eytanadler can you check that this runs on your MPhys case with child DVGeos?

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 0% with 26 lines in your changes are missing coverage. Please review.

Project coverage is 65.45%. Comparing base (d3f5e6d) to head (19f5d7d).

Files	Patch %	Lines
pygeo/mphys/mphys_dvgeo.py	0.00%	26 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #239 +/- ## ========================================== + Coverage 65.44% 65.45% +0.01% ========================================== Files 47 47 Lines 12211 12208 -3 ========================================== Hits 7991 7991 + Misses 4220 4217 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

eytanadler commented 5 months ago

@eytanadler can you check that this runs on your MPhys case with child DVGeos?

It seems to work for my case

lamkina commented 5 months ago

@mdolab/pygeo_maintainers this is ready to merge, but I don't have the permissions. Can one of you take care of it?

mdolab / pygeo