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

Adding a dynamic stress (wave model) to wall_models #1233

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

ayalamanuel
Copy link

@ayalamanuel ayalamanuel commented Sep 4, 2024

Summary

We have introduced a dynamic stress as a wall model. the dynamic stress calculates the stress due to waves. a new struct was implemented in ShearStressSimple.H where the wave stress is calculated. a new header (MOSD.H) was created with the calculations of the stress. the WallFunction.cpp has been modified accordingly.

Unit testing (test_mosd.cpp) and regression testing (channel_mosd.inp) using CPU has been done and added. The documentation of the model has also been added in the inputs_Boundary_conditions.rst file.

Pull request type

Please check the type of change introduced:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Checklist

The following is included:

  • new unit-test(s)
  • new regression test(s)
  • documentation for new capability

This PR was tested by running:

  • the unit tests
    • on GPU
    • on CPU
  • the regression tests
    • on GPU
    • on CPU

Additional background

Issue Number:

Copy link
Contributor

@marchdf marchdf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick list of things to do (I am sure you are working on these):

  • documentation
  • reg test
  • unit test

I can look at the PR in detail later this week. Thank you for working on this!

@gdeskos gdeskos self-requested a review September 4, 2024 19:57
@marchdf
Copy link
Contributor

marchdf commented Sep 11, 2024

@ayalamanuel do you have any updates on this?

@ayalamanuel
Copy link
Author

@marchdf Old files have been updated. Unit test, regression test and documentation have been added. Thanks!

amr-wind/boundary_conditions/wall_models/MOSD.H Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/wall_models/MOSD.H Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/wall_models/MOSD.H Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/wall_models/MOSD.H Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/wall_models/WallFunction.cpp Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/wall_models/WallFunction.cpp Outdated Show resolved Hide resolved
unit_tests/boundary_conditions/test_mosd.cpp Outdated Show resolved Hide resolved
@ayalamanuel
Copy link
Author

I've made the modifications to all the comments made @marchdf

docs/sphinx/user/inputs_Boundary_conditions.rst Outdated Show resolved Hide resolved
unit_tests/boundary_conditions/test_mosd.cpp Outdated Show resolved Hide resolved
unit_tests/boundary_conditions/test_mosd.cpp Outdated Show resolved Hide resolved
@marchdf
Copy link
Contributor

marchdf commented Sep 18, 2024

A meta comment here: to get past the CI check, the first thing you need to do is format your code. You can do that automatically with clang-format: https://exawind.github.io/amr-wind/developer/coding_guidelines.html#style-guide-recommendations

submods/amrex Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@ayalamanuel ayalamanuel force-pushed the manuel/wall_model branch 2 times, most recently from 7489570 to 38bdbe7 Compare September 20, 2024 18:14
Copy link
Contributor

@moprak-nrel moprak-nrel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the interpolation logic looks a lot better! Here's a few more minor comments on the interpolation

amr-wind/boundary_conditions/wall_models/WallFunction.cpp Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/wall_models/WallFunction.cpp Outdated Show resolved Hide resolved
tau.get_shear(
vv, wspd, u_dx, v_dx, xc, 1) /
mu * den(i, j, k);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious, when do we expect this branch to occur?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking through the changes, did this get resolved?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it did get solve. Or perhaps I am not understanding the issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to figure out when this branch will occur with all the new interpolation changes.

@marchdf
Copy link
Contributor

marchdf commented Oct 9, 2024

Can I do anything to help push this through? We are super close I think.

@ayalamanuel
Copy link
Author

@marchdf @moprak-nrel sorry for taking so long with this! Hopefully it's almost finished

@marchdf
Copy link
Contributor

marchdf commented Oct 14, 2024

can you resolve those documentation conflicts?

@ayalamanuel
Copy link
Author

seems like the documentation issues are because I used and outdated theory.rst file. How can I update that file in my branch? then I can just add the once again the new part of documentation from my implementation

@moprak-nrel
Copy link
Contributor

seems like the documentation issues are because I used and outdated theory.rst file. How can I update that file in my branch? then I can just add the once again the new part of documentation from my implementation

I would rebase your branch with the latest amr-wind main and resolve any conflicts there. You'll need to do this before the PR is ready for merging anyway.

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.

3 participants