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

Inflow-outflow boundary condition #1066

Merged
merged 84 commits into from
Aug 21, 2024
Merged

Inflow-outflow boundary condition #1066

merged 84 commits into from
Aug 21, 2024

Conversation

mukul1992
Copy link
Contributor

@mukul1992 mukul1992 commented May 18, 2024

Summary

Enabling the mass_inflow_outflow (MIO) BC which allows for both inflow and outflow cells in a boundary.

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 (macOS M3, gcc-13)

Additional background

This functionality would be required for coupling with ERF and was started after discussions with @gdeskos and @mchurchf, along with @asalmgren, as it was a desired functionality within AMR-Wind too. Additionally, @hgopalan has expressed interest, and @marchdf has helped along the way.

Description

The changes required can be organized into six categories:

  1. adding MIO as one of the BCs, mapping it to a custom math BC, and adding the mechanics required for enabling UDFs.
  2. the new TwoLayer UDF for testing, currently it implements a simple two-layer x-direction flow in opposite directions as a test, but this UDF can be modified to test progressively more complex cases, finishing with a regression test case such as vortex initialization and advection across the boundary.
  3. the custom Neumann function for filling the outflow boundary cells in MassInflowOutflowBC.cpp
  4. adding the inflow and outflow conditions in the WENO upwinding functions to map to ext_dir and foextrap behavior respectively.
  5. calling the enforceSolvability routine in AMReX-Hydro before the mac-projection, which adjusts the outflux to match with influx (will link to the corresponding PR in Hydro).
  6. the cmake scripts and input files for testing, these can be reorganized and cleaned up ultimately into a proper regression test as development completes. The input file being used for testing is input_inout.

@marchdf marchdf marked this pull request as draft May 20, 2024 14:24
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.

Made a first quick pass.

amr-wind/boundary_conditions/MassInflowOutflowBC.cpp Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/MassInflowOutflowBC.cpp Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/BCInterface.H Outdated Show resolved Hide resolved
amr-wind/boundary_conditions/scalar_bcs.H Outdated Show resolved Hide resolved
amr-wind/convection/incflo_godunov_ppm.H Outdated Show resolved Hide resolved
cmake/amr-wind-utils.cmake Outdated Show resolved Hide resolved
amr-wind/physics/udfs/TwoLayer.H Outdated Show resolved Hide resolved
amr-wind/physics/udfs/TwoLayer.cpp Outdated Show resolved Hide resolved
amr-wind/projection/incflo_apply_nodal_projection.cpp Outdated Show resolved Hide resolved
bctest/input_inflow Outdated Show resolved Hide resolved
@marchdf
Copy link
Contributor

marchdf commented May 22, 2024

Most of my comments right now are just noting the obvious things to clean up before actually merging. Thank you for taking this task on!!

@mbkuhn
Copy link
Contributor

mbkuhn commented May 22, 2024

Speaking of merging, it looks like there are some complicated merge conflicts between this branch and main. It may be helpful to rebase your branch to upstream/main to remove some of those conflicts and get the unrelated parts of this branch up-to-date. Removing the merge conflicts will also allow the CI tests to run.

@mukul1992
Copy link
Contributor Author

@marchdf Yes, I will certainly clean up all the debug statements and incorporate other changes you recommended.

@mbkuhn Yes, I will do the rebase over the next week along with cleaning things up.

Thanks for the early feedback!

@mukul1992
Copy link
Contributor Author

PS. I have been able to rebase this code with the latest changes in the Exawind/amr-wind repo. I have also updated the amrex submod in the index (let me know if I did it incorrectly). And have updated the BCType from user_1 to direction_dependent in the code.
And finally, here's the draft PR for AMReX-Hydro that includes the outflow-correction for enforcing solvability.

@mukul1992
Copy link
Contributor Author

@asalmgren @marchdf @mbkuhn
I have cleaned up the code and addressed prior comments (received via github or discussed verbally).

There are two sticking point: the solution not reaching steady state for my test and the ctest failing (which I can run manually).

But I think the code is ready to be reviewed while we debug these.

amr-wind/boundary_conditions/MassInflowOutflowBC.cpp Fixed Show resolved Hide resolved
amr-wind/boundary_conditions/MassInflowOutflowBC.cpp Fixed Show resolved Hide resolved
amr-wind/boundary_conditions/scalar_bcs.cpp Fixed Show resolved Hide resolved
amr-wind/boundary_conditions/velocity_bcs.H Fixed Show resolved Hide resolved
@mukul1992
Copy link
Contributor Author

I was able to correct the issue that was causing my in-out BC ctest to fail in 5856a85.
While looking at this, I noticed a new issue related to the MAC velocity valid cell fills at the outflow for this BC, which did not exist earlier. I'll be working on that next week.

@mukul1992 mukul1992 requested a review from marchdf August 20, 2024 17:58
@marchdf marchdf changed the title DRAFT PR: inflow-outflow boundary condition Inflow-outflow boundary condition Aug 20, 2024
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.

Just some minor code suggestions you can accept. Also, can you add the new BC type to the docs? Around here: https://github.com/Exawind/amr-wind/blob/main/docs/sphinx/user/inputs_Boundary_conditions.rst?plain=1#L12 and then below the existing mass_inflow BC you can add more details.

amr-wind/core/FieldFillPatchOps.H Outdated Show resolved Hide resolved
amr-wind/core/FieldFillPatchOps.H Outdated Show resolved Hide resolved
@mukul1992 mukul1992 marked this pull request as ready for review August 20, 2024 18:58
mukul1992 and others added 2 commits August 20, 2024 12:02
Co-authored-by: Marc T. Henry de Frahan <marchdf@gmail.com>
auto-merge was automatically disabled August 21, 2024 16:29

Head branch was pushed to by a user without write access

@marchdf marchdf enabled auto-merge (squash) August 21, 2024 16:47
@marchdf marchdf merged commit 3e23581 into Exawind:main Aug 21, 2024
13 of 14 checks passed
mbkuhn pushed a commit to mbkuhn/amr-wind that referenced this pull request Aug 22, 2024
---------

Co-authored-by: Marc T. Henry de Frahan <marchdf@gmail.com>
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.

4 participants