Require the full inflow state when grcbc_in is enabled - #1854
Open
sbryngelson wants to merge 2 commits into
Open
Require the full inflow state when grcbc_in is enabled#1854sbryngelson wants to merge 2 commits into
sbryngelson wants to merge 2 commits into
Conversation
A GRCBC subsonic inflow relaxes the boundary towards a prescribed state, so that state has to be given in full: velocity, pressure, partial densities and volume fractions. The validator checked only that the boundary type was -7. Leaving alpha_rho_in and alpha_in unset does not fail; the boundary keeps the default sentinel and the solution diverges over a few hundred steps, surfacing as an ICFL abort far from the cause. That cost me two runs before I compared against a case that worked, which is a poor way to learn that four parameters are mandatory rather than three. The check accepts every example case in the tree and rejects exactly the configuration that diverged. Claude-Session: https://claude.ai/code/session_01HMJ7cycfo7kTFSFq5yhHLG
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Tightens input validation for GRCBC subsonic inflow so that the full prescribed inflow state is required when grcbc_in is enabled, preventing late-run divergence caused by unset sentinel defaults.
Changes:
- Add validation that
pres_in,vel_in,alpha_rho_in(*), andalpha_in(*)are all set whengrcbc_inis active. - Improve failure reporting by listing exactly which inflow-state parameters are missing.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # full. An unset component keeps its default sentinel and the boundary diverges over a few hundred | ||
| # steps rather than failing outright, which is a hard failure to read backwards from an ICFL abort. | ||
| num_fluids = self.get("num_fluids", 1) | ||
| missing = [n for n in (f"bc_{dir}%pres_in", f"bc_{dir}%vel_in(1)") if self.get(n) is None] |
Comment on lines
+1800
to
+1802
| missing = [n for n in (f"bc_{dir}%pres_in", f"bc_{dir}%vel_in(1)") if self.get(n) is None] | ||
| missing += [f"bc_{dir}%alpha_rho_in({i})" for i in range(1, num_fluids + 1) if self.get(f"bc_{dir}%alpha_rho_in({i})") is None] | ||
| missing += [f"bc_{dir}%alpha_in({i})" for i in range(1, num_fluids + 1) if self.get(f"bc_{dir}%alpha_in({i})") is None] |
|
Claude Code Review Head SHA: 4dae9bb Files changed:
Findings:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1854 +/- ##
=======================================
Coverage 61.26% 61.26%
=======================================
Files 84 84
Lines 22330 22330
Branches 3265 3265
=======================================
Hits 13680 13680
Misses 6207 6207
Partials 2443 2443 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A GRCBC subsonic inflow relaxes the boundary towards a prescribed state, so that state has to be given in full: velocity, pressure, partial densities and volume fractions. The validator checked only that the boundary type was
-7.Leaving
alpha_rho_inandalpha_inunset does not fail. The boundary keeps its default sentinel and the solution diverges over a few hundred steps, which surfaces as an ICFL abort a long way from the cause:Flat, then exponential. That cost me two runs before I compared against a case that worked, which is a poor way to learn that four parameters are mandatory rather than three.
The new check accepts every example case in the tree (
./mfc.sh precheckpasses, including "Validating example cases") and rejects exactly the configuration that diverged:alpha_rho_in/alpha_inremovedSubsonic Inflow (grcbc_in) needs the full inflow state; missing bc_x%alpha_rho_in(1), bc_x%alpha_in(1)https://claude.ai/code/session_01HMJ7cycfo7kTFSFq5yhHLG