Skip to content

Comments

Fix direction-dependent BC#164

Merged
asalmgren merged 7 commits intoAMReX-Fluids:developmentfrom
mbkuhn:inout_solvability_fix
Apr 10, 2025
Merged

Fix direction-dependent BC#164
asalmgren merged 7 commits intoAMReX-Fluids:developmentfrom
mbkuhn:inout_solvability_fix

Conversation

@mbkuhn
Copy link
Contributor

@mbkuhn mbkuhn commented Apr 9, 2025

No description provided.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Apr 9, 2025

@marchdf @moprak-nrel

see the code change and my comments on the linked PR and issue. I'm going to do some more investigating to see if there is a better fix, because I have my doubts. But this definitely shows a bug in the current implementation.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Apr 9, 2025

The problem becomes clear when you look here: dhi is calculated using domain, which is passed in, which only considers the level = 0 indices. Then dhi is checked against the indices at each level (not just level = 0). As a result, the masks are calculated at the wrong cells. Then, the correct_outflow step takes place and uses the same domain-based check, having the potential to change velocity values in the middle of the domain (on lev > 0).

https://github.com/AMReX-Fluids/AMReX-Hydro/pull/164/files#diff-dee99f19e44146dd0e710bd7a69d1d5a6cece50a3d7cb3d970a59ae862637c20R43

@asalmgren asalmgren self-requested a review April 9, 2025 15:17
@marchdf
Copy link
Contributor

marchdf commented Apr 9, 2025

Nice work!

@mbkuhn mbkuhn marked this pull request as ready for review April 9, 2025 22:58
@@ -59,6 +63,8 @@ void set_inout_masks(
for (MFIter mfi(*vel_mf, TilingIfNotGPU()); mfi.isValid(); ++mfi) {

Box box = mfi.tilebox();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, I wouldn't use a level_mask array, I would just check if this box is covered by finer cells and skip it entirely. However, I don't know if that is possible, and I'm familiar with makeFineMask from the AMR-Wind context, so that's what I used.

Copy link
Contributor

Choose a reason for hiding this comment

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

@WeiqunZhang thoughts on this one?

@asalmgren asalmgren merged commit 43f12ba into AMReX-Fluids:development Apr 10, 2025
10 checks passed
@mbkuhn mbkuhn deleted the inout_solvability_fix branch April 10, 2025 17:42
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