Add WENO/MUSCL parameter guards with corresponding recon_type#1391
Add WENO/MUSCL parameter guards with corresponding recon_type#1391Cowsreal wants to merge 15 commits intoMFlowCode:masterfrom
Conversation
…de $:GPU_ROUTINE macro
…responding recon_type check for check_muscl_simulation
… into minorChanges
Code Review by Qodo
1. Overblocks viscous flags
|
📝 WalkthroughWalkthroughThe changes implement reconstruction-mode-specific parameter validation and filtering across the codebase. The validator now enforces mutual exclusivity between MUSCL and WENO reconstruction modes: MUSCL parameters are disallowed when 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 614490c0-438f-46c6-8a88-5bb09f2183aa
📒 Files selected for processing (3)
toolchain/mfc/case_validator.pytoolchain/mfc/test/case.pytoolchain/mfc/test/cases.py
| recon_type = self.get("recon_type", 1) | ||
| # WENO_TYPE = 1 | ||
| if recon_type != 1: | ||
| return |
There was a problem hiding this comment.
Validate recon_type domain before mode-specific early returns.
Line 736 and Line 775 return early for non-target modes, but there is no explicit guard that recon_type must be 1 or 2. Values like recon_type=3 can bypass both WENO and MUSCL validation paths silently.
Suggested fix
def check_weno(self):
"""Checks constraints regarding WENO order"""
recon_type = self.get("recon_type", 1)
+ self.prohibit(recon_type not in [1, 2], "recon_type must be 1 (WENO) or 2 (MUSCL)")
# WENO_TYPE = 1
if recon_type != 1:
return
@@
def check_muscl(self):
"""Check constraints regarding MUSCL order"""
recon_type = self.get("recon_type", 1)
+ self.prohibit(recon_type not in [1, 2], "recon_type must be 1 (WENO) or 2 (MUSCL)")
int_comp = self.get("int_comp", "F") == "T"Also applies to: 773-776
|
Persistent review updated to latest commit 9b2af1d |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1391 +/- ##
=======================================
Coverage 64.75% 64.76%
=======================================
Files 71 71
Lines 18716 18716
Branches 1548 1548
=======================================
+ Hits 12120 12121 +1
+ Misses 5641 5640 -1
Partials 955 955 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Description
Add various guards for cases running MUSCL/WENO reconstruction with their corresponding recon_type value. Previously, this wasn't checked very tightly, and setting recon_type=2 while having WENO parameters such as weno_order, weno_eps, etc set. This would be overrided by the recon_type parameter and result in a MUSCL order 0 run.
Type of change
AI code reviews
Reviews are not triggered automatically. To request a review, comment on the PR:
@coderabbitai review— incremental review (new changes only)@coderabbitai full review— full review from scratch/review— Qodo review/improve— Qodo code suggestions@claude full review— Claude full review (also triggers on PR open/reopen/ready)claude-full-review— Claude full review via label