Improved cavitation constraint

[yqliaohk]

Purpose

This PR includes changes to cavitation constraint. I modified and reorganized it so it is backward-compatible with the original form while also can be used for the formulation I used in my thesis.

Type of change

• New feature (non-breaking change which adds functionality)

Testing

• Backward-compatibility test: I first retrained the test_cl_solve.py:TestSolve test without updated code to generate the new ref file that include cavitation as a target function (with computecavitation True and cavitation added as one of evalfuncs). Then I used the updated code to run the reg test. The reg test passed.
• Complex step test 1: I used the default options with updated code to retrain the ref file adjoint_rans_tut_wing.json (with computecavitation set to True and cavitation added as one of evalfuncs), and then ran complex step test. The test passed.
• Complex step test 2: Same test as the previous one, but with cavExponent set to 2, which is the formulation I used in my thesis.
• I reran one of my previous hydro-only case and the evaluated cavitation function value matched 7 digits with my previous result.

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

[codecov]

Codecov Report

Merging #176 (7f33ba6) into main (4b179a0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #176   +/-   ##
=======================================
  Coverage   41.04%   41.04%           
=======================================
  Files          13       13           
  Lines        3793     3793           
=======================================
  Hits         1557     1557           
  Misses       2236     2236           

Impacted Files	Coverage Δ
adflow/pyADflow.py	67.90% <ø> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[gawng]

Just so we're on the same page and it is documented here since it came up in the maintenance meeting, @yqliaohk what are the remaining action items to do before this branch can be merged?
I believe the last time we spoke, you said you want some regression tests added. Maybe it would also be good for me to have a completed optimization using the cavitation, though my derivatives are all agreeing with the FD approximations from SNOPT.

[yqliaohk]

Just so we're on the same page and it is documented here since it came up in the maintenance meeting, @yqliaohk what are the remaining action items to do before this branch can be merged? I believe the last time we spoke, you said you want some regression tests added. Maybe it would also be good for me to have a completed optimization using the cavitation, though my derivatives are all agreeing with the FD approximations from SNOPT.

No action items remaining now. Once I clean the ref files, it will be ready to merge.

[gawng]

The implementation works as expected

[anilyil]

I looked at using heaviside for cavitation and separation, and I am not convinced if its the best approach. We can probably do better with a KS-based cavitation constraint. I am fine with merging this PR after I review it so that @yqliaohk has these changes in the adflow history for her thesis and paper cases. We can discuss further on the KS based implementation.

[ewu63]

I personally think we should merge stuff instead of letting stuff sit stale since I assume further development of this will not be anyone's priority. @anilyil what do you think?

[gawng]

I talked with Anil about a KS-based cavitation constraint and its definitely something we'll work on getting merged when its ready. It can potentially be a second option for the cavitation constraint if we decide there are specific pros and cons of each method for constraining cavitation. Right now though, i also agree we should just try to merge yingqians code.

[sseraj]

Once I clean the ref files, it will be ready to merge.

What's the status on this? It looks like there was a lot of effort put into testing, so it would be nice to have that added to the repo.

Also, the docs build is failing because each new option needs a description in options.yaml. This would be the place to put things like this

cavExponent set to 2, which is the formulation I used in my thesis

[yqliaohk]

Once I clean the ref files, it will be ready to merge.

What's the status on this? It looks like there was a lot of effort put into testing, so it would be nice to have that added to the repo.

Also, the docs build is failing because each new option needs a description in options.yaml. This would be the place to put things like this

cavExponent set to 2, which is the formulation I used in my thesis

@sseraj Were you talking about adding all the testing I did in the description to the repo?

[sseraj]

@sseraj Were you talking about adding all the testing I did in the description to the repo?

No, not all of it, just whatever you had mind.

[gawng]

Thanks for getting this in! The tests pass and the changes look good to me.