Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SA rough #259

Draft
wants to merge 85 commits into
base: main
Choose a base branch
from
Draft

SA rough #259

wants to merge 85 commits into from

Conversation

DavidAnderegg
Copy link
Collaborator

@DavidAnderegg DavidAnderegg commented Jan 30, 2023

Purpose

This extends the SA model for rough walls as described here.

How to use

To activate the rough variant, those ADflow options must be present:

"useRoughSA": True,
"useBlockette": False,

The surface roughness may be prescribed using cgns_utils overwriteBC with the following example bcfile:

4 kLow bcwall le BCDataSet_1 BCWall Dirichlet SandGrainRoughness 1e-3

Alternatively, the ADflow-method setSurfaceRoughness(self, ks, groupName=None) may be used.

Known bugs

The proposed changes work, but two bugs are still present:

  • The effect of the specified surface-roughness is to little compared to SU2:

  • The effect of the specified surface-roughness is to little compared to experiments:

  • The predicted shape of the effect seems correct.

  • The AD-gradients only agree to a relative tolerance of ~1e-8 compared to complex step.

All of this information may be found to more detail in the report i had to write.

Expected time until merged

This depends on the mdo-team. Although there are two bugs, I would argue this feature is already usable. I spent a lot of time trying to find the errors, but i was unsuccessful. My intention is to do something else and later come back and fix it.

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

I used the tutorial wing and roughened its surface. Based on this, I introduced three tests:

  • Add a new instance to test_adjoint.py with the rough wing.
  • Add a new instance to test_functionals.py with the rough wing.
  • Add a new instace to test_functionals.py where the rough SA variant is activated. But the restart and reference files are from the test rans_tut_wing. This makes sure SA rough with a roughness value of 0 is consistent to the regular SA model.

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

@gawng
Copy link
Contributor

gawng commented Sep 23, 2024

Hi @DavidAnderegg , I was just curious if there were any specific things you wanted to finish before making this PR ready to review? @sabakhshi said he had tried out the SA model for analysis.

@DavidAnderegg
Copy link
Collaborator Author

DavidAnderegg commented Sep 24, 2024

Hi @gawng

As written in the description, there is still that problem where the effect of the specified sand grain roughness is to little compared to SU2 and experiments. Unfortunately, I was not able to determine the cause yet.

If you guys are ok with that, this should be ready to merge.

@gawng
Copy link
Contributor

gawng commented Sep 30, 2024

@DavidAnderegg I brought up this PR at the maintenance meeting today, and I think what would be good is to proceed with updating and merging this PR. But please do put a code warning for SA rough about the behavior you were seeing wrt lower surface roughness effects compared to experiments and SU2. As long as it is documented what issues there are with SA rough, having the capability in ADflow is better than letting this branch of code go stale.

@DavidAnderegg
Copy link
Collaborator Author

Hi @gawng, sounds good. I will dust it off in the coming days..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants