fixing petsc output vector error in the MPhys wrapper

[ArshSaja]

Purpose

In rev, when the adjoint function is called, there is some issue with f2py handling some outputs in the function:. For example, d_residuals["adflow_states"] is not passing the correct output from the Fortran layer to Python in this call: solver.adflow.adjointapi.solveadjoint(d_outputs["adflow_states"], d_residuals["adflow_states"], True). Thus, Some derivatives become wrong:

Full Model: 'cruise0.coupling.prop.Fn' wrt 'geo_dvs.Nacelle:XSecCurve_1:Circle_Diameter'
    Analytic Magnitude: 2.563607e+02
          Fd Magnitude: 4.547066e+02 (fd:None)
    Absolute Error (Jan - Jfd) : 1.983459e+02 *

    Relative Error (Jan - Jfd) / Jfd : 4.362064e-01 *
    MPI Rank 8

    Raw Analytic Derivative (Jfor)
[[-256.36070479]]

    Raw FD Derivative (Jfd)
[[-454.70664024]]

This PR address this issue with a minor fix. It fixes the derivative issues.

  Full Model: 'cruise0.coupling.prop.Fn' wrt 'geo_dvs.Nacelle:XSecCurve_1:Circle_Diameter'
    Analytic Magnitude: 4.556590e+02
          Fd Magnitude: 4.547229e+02 (fd:None)
    Absolute Error (Jan - Jfd) : 9.361134e-01 *

    Relative Error (Jan - Jfd) / Jfd : 2.058646e-03 *
    MPI Rank 0

    Raw Analytic Derivative (Jfor)
[[-455.6590081]]

    Raw FD Derivative (Jfd)
[[-454.72289467]]

Expected time until merged

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

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

[anilyil]

@lamkina can you test this with your code as well?

[codecov]

Codecov Report

Merging #300 (a1c6509) into main (4dda7b3) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #300      +/-   ##
==========================================
- Coverage   42.01%   41.98%   -0.03%     
==========================================
  Files          13       13              
  Lines        3999     4001       +2     
==========================================
  Hits         1680     1680              
- Misses       2319     2321       +2     

Files Changed	Coverage Δ
adflow/mphys/mphys_adflow.py	0.00% <0.00%> (ø)

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

[anilyil]

Do you really need both of the copies? I would imagine the openmdao vector is already doing one copy when setting the values? Did you test that the second copy is strictly necessary?

Also, any idea why just passing the pointer did not work? Maybe we can get rid of all of the copies?

[ArshSaja]

Do you really need both of the copies? I would imagine the openmdao vector is already doing one copy when setting the values? Did you test that the second copy is strictly necessary?

Also, any idea why just passing the pointer did not work? Maybe we can get rid of all of the copies?

Actually, the second copy is not necessary. I just did a copy for consistency. The first one is necessary though. Just setting the pointer didn't work. I am okay with removing it too

[anilyil]

Remove the second copy if its not strictly necessary. Try to understand why the first copy is needed. We may be able to get away with not copying it. Might need to slightly modify the f2py wrapper or add a new wrapper function that takes in the openmdao vector.

[anilyil]

To avoid further delaying this fix, I say this: test it again with the current state. Make sure all relevant cases work. Then, we can merge this PR and create an issue to investigate the removal of the first copy later.

[ArshSaja]

Remove the second copy if its not strictly necessary. Try to understand why the first copy is needed. We may be able to get away with not copying it. Might need to slightly modify the f2py wrapper or add a new wrapper function that takes in the openmdao vector.

I fixed it. I think this is the only place we are using copy, for now, we can just do the copy may be. later if it's necessary, we can just add a helper function to set them properly.

[ArshSaja]

To avoid further delaying this fix, I say this: test it again with the current state. Make sure all relevant cases work. Then, we can merge this PR and create an issue to investigate the removal of the first copy later.

It tested and works fine with the aeroprop cases!