Write vel(2) to 2D probe files and drop the per-step print - #1844
Write vel(2) to 2D probe files and drop the per-step print#1844sbryngelson wants to merge 2 commits into
Conversation
For a plain 2D case (no bubbles, no hypoelasticity) `s_write_probe_files` wrote only (time, rho, vel(1), pres), silently dropping the second velocity component, and also `print`ed a line to stdout on every time step for every probe. The 3D branch writes all three velocity components, and the 2D bubble and hypoelastic branches write vel(2), so this was the plain-2D branch being incomplete rather than a deliberate choice. The stray `print` floods the job log of any long run. Note this changes the 2D probe file from four columns to five, so 2D probe golden files need regenerating. Fixes #1841 Claude-Session: https://claude.ai/code/session_01HMJ7cycfo7kTFSFq5yhHLG
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
This PR fixes incomplete probe output for plain 2D simulations by writing the second velocity component (vel(2)) to probe files and removing a per-step print that floods stdout/logs.
Changes:
- Update the plain-2D probe output to include
vel(2)(expanding output from 4 to 5 columns). - Remove per-probe, per-timestep stdout logging from
s_write_probe_files. - Add a fypp guard to avoid referencing
vel(2)under 1D case optimization.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| #:if not MFC_CASE_OPTIMIZATION or num_dims > 1 | ||
| write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8)') nondim_time, rho, vel(1), vel(2), pres | ||
| #:endif |
|
Claude Code Review Head SHA: aa71e84 Files changed:
Findings:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1844 +/- ##
=======================================
Coverage 61.26% 61.26%
=======================================
Files 84 84
Lines 22330 22329 -1
Branches 3265 3265
=======================================
Hits 13680 13680
+ Misses 6207 6206 -1
Partials 2443 2443 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Now exercised at runtime, which I had flagged above as missing. 2D case (flat plate in a freestream, 3 probes,
Reminder for whoever merges: 2D probe golden files need regenerating, since the file goes from four columns to five. |
Lines of Code
|
Fixes #1841.
For a plain 2D case (no bubbles, no hypoelasticity),
s_write_probe_fileswrote only(time, rho, vel(1), pres)— the second velocity component was dropped — and it alsoprinted a line to stdout on every time step for every probe:The 3D branch writes all three components and the 2D bubble and hypoelastic branches both write
vel(2), so this looks like the plain-2D branch being incomplete rather than intentional. Theprintfloods the log of any long run: one line per probe per step.The fypp guard matches the neighbouring branches, since
vel(2)does not exist under 1D case optimization.Heads up for reviewers: this changes the 2D probe file from four columns to five, so any 2D probe golden files need regenerating. I have not run a 2D probe case with this patch — I hit the missing component while post-processing and fixed it, but my later 2D runs used no probes and my 3D runs take a different branch. Worth a quick check before merge.
./mfc.sh precheckpasses.https://claude.ai/code/session_01HMJ7cycfo7kTFSFq5yhHLG