Skip to content

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Nov 29, 2025

  • Add bubble_probe_ready flag to prevent ICFL calculation before bubble initialization
  • Relax ICFL checks (NaN/Infinity) for bubble simulations during startup
  • Fix 2D/3D probe output format string mismatches
  • Skip probe writing until bubble data is stable
  • Initialize bubble probe after first RK stage for bubble cases

Summary by CodeRabbit

  • Bug Fixes

    • Improved bubble probe readiness to delay probe I/O until initialization and added safeguards (e.g., division-by-zero protection) in bubble-related outputs.
    • Fixed internal-energy startup updates.
  • Refactor

    • Simplified probe output API (acceleration magnitude removed).
    • Switched how offsets are specified when writing output to include explicit dimension counts.
  • New Tools

    • Added a golden-file comparison script and summary report; multiple golden test files regenerated.

✏️ Tip: You can customize this high-level summary in your review settings.

- Add bubble_probe_ready flag to prevent ICFL calculation before bubble initialization
- Relax ICFL checks (NaN/Infinity) for bubble simulations during startup
- Fix 2D/3D probe output format string mismatches
- Skip probe writing until bubble data is stable
- Initialize bubble probe after first RK stage for bubble cases
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 29, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds bubble-probe readiness gating and a marker routine, updates probe-writing signature, replaces Silo offset option calls with count-aware variants, refactors internal-energy indexing, adjusts speed-of-sound guards, and refreshes many test golden metadata and numeric golden files across the repo.

Changes

Cohort / File(s) Summary
Silo API dimensionality updates
src/post_process/m_data_output.fpp
Replace DBADDIOPT(optlist, DBOPT_LO_OFFSET, lo_offset) / DBADDIOPT(..., DBOPT_HI_OFFSET, hi_offset) with DBADDIAOPT(optlist, ..., N, offset) using N=3/2/1 per grid dimension branches.
Bubble probe readiness & probe I/O
src/simulation/m_data_output.fpp, src/simulation/m_time_steppers.fpp
Add global bubble_probe_ready flag, s_mark_bubble_probe_ready() public routine, gate probe file creation/writes/close on readiness, update time-stepper to set readiness after second ICFL for Eulerian bubbles.
Probe write signature propagation
src/simulation/m_derived_variables.fpp
Remove accel_mag from s_write_probe_files call to match updated signature s_write_probe_files(t_step, q_cons_vf).
Internal energy indexing refactor
src/simulation/m_start_up.fpp
Replace offset-based indices (intxb/advxb/contxb) with explicit public index descriptors (internalEnergies_idx%beg, adv_idx%beg, cont_idx%beg) and use fluid_pp(i)%gamma/pi_inf/qv in per-fluid internal-energy initialization.
Speed-of-sound denominator guard
src/common/m_variables_conversion.fpp
Introduce local denom, use max(rho, small_alf) and max(1 - adv(num_fluids), small_alf) guards to avoid division-by-zero in certain model branches.
Golden metadata refresh (many tests)
tests/*/golden-metadata.txt (many files)
Update timestamps, invocation flags, CMake versions, compiler/toolchain paths (shift to AppleClang/Homebrew gfortran), OpenMP toggles, Fypp paths, and CPU descriptors (Apple M1/M1 Max) across 20+ test metadata files.
Golden data updates (multiple tests)
tests/*/golden.txt (many files)
Update numeric reference data: adjusted floating-point values, added 050-suffixed data blocks in several tests, and other precision/sequence changes across cons/prim/probe data files.
Golden comparison tooling & results
compare_goldenfiles.py, golden_comparison_results.txt
Add script to compare local vs repo golden files (parallel execution, numeric-tolerance checks) and include aggregated comparison results report showing OK/DIFF/SKIP/ERROR counts and max-difference detail.
Various test golden data deltas
tests/**/golden.txt (specific cases listed in raw summary)
Numerous per-test numeric-only updates (precision changes, added blocks, small-value adjustments) — data-only edits without API changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas to review closely:

  • src/post_process/m_data_output.fpp: verify correct N mapping (1/2/3) and Silo API compatibility.
  • src/simulation/m_data_output.fpp & m_time_steppers.fpp: initialization sequencing for bubble_probe_ready, ensure no probe I/O before FD allocation and readiness is consistently propagated.
  • Call sites of s_write_probe_files: ensure all callers updated to new signature.
  • src/simulation/m_start_up.fpp: correctness of index-beg substitutions vs previous offset logic.
  • src/common/m_variables_conversion.fpp: numerical stability and unit tests for speed-of-sound changes.
  • Large number of golden file changes: confirm intentional recomputation vs spurious drift.

Suggested labels

Review effort 4/5, size:XXL

Suggested reviewers

  • wilfonba

Poem

🐰 I waited by the data stream, then tapped my little paw,
Probes slept until the bubbles grew and all the arrays saw.
Silo counts now tell the dims, indices hop in line,
Golden numbers shimmer bright — the tests will soon be fine.
Hooray — a rabbit’s tiny cheer for code that runs on time!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete; it lacks most required template sections including Type of change, Scope, How Has This Been Tested, and Checklist items. Fill out the full PR description template including type of change, scope checkbox, testing details, and all checklist items to meet repository requirements.
Title check ❓ Inconclusive The title 'Probe goldenfiles?' is vague and generic, using a question mark without clearly conveying the main changes described in the PR. Revise the title to be more descriptive and specific, such as 'Add bubble_probe_ready flag and fix probe output format issues' or similar.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Nov 29, 2025

Codecov Report

❌ Patch coverage is 34.73684% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.33%. Comparing base (e2ce07d) to head (8e8d5fb).

Files with missing lines Patch % Lines
src/simulation/m_data_output.fpp 29.26% 57 Missing and 1 partial ⚠️
src/simulation/m_start_up.fpp 0.00% 3 Missing ⚠️
src/simulation/m_time_steppers.fpp 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1069      +/-   ##
==========================================
- Coverage   44.36%   44.33%   -0.03%     
==========================================
  Files          71       71              
  Lines       20590    20608      +18     
  Branches     1994     1996       +2     
==========================================
+ Hits         9134     9137       +3     
- Misses      10310    10326      +16     
+ Partials     1146     1145       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbryngelson sbryngelson marked this pull request as ready for review November 29, 2025 23:24
Copilot AI review requested due to automatic review settings November 29, 2025 23:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a bubble_probe_ready flag to prevent premature ICFL calculations before bubble initialization, ensuring probe output correctness for bubble simulations.

  • Adds bubble_probe_ready flag to delay ICFL checks until after bubble initialization
  • Fixes probe output format string mismatches between 2D and 3D cases
  • Initializes bubble probe after first RK stage for bubble simulations

Reviewed changes

Copilot reviewed 63 out of 1058 changed files in this pull request and generated no comments.

File Description
src/simulation/m_time_steppers.fpp Adds bubble probe ready marking after second ICFL calculation, reorders variable declaration comment
tests/*/golden.txt Updates golden file numerical data (minor floating-point variations)
tests/*/golden-metadata.txt Updates test metadata with new timestamps, build configurations, and system information

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
tests/0BD42F35/golden-metadata.txt (1)

1-119: I'm unable to access the repository files due to a clone failure in the sandbox environment. However, I can analyze the concern based on the information provided in the review comment and established testing best practices.

Let me reason through the verification using the available context:

Analysis of the Golden Metadata File:

The review comment correctly identifies that the golden-metadata.txt file contains entirely environmental configuration details:

  • Build timestamps and machine specifications
  • Compiler versions and paths (AppleClang, GNU Fortran)
  • CMake version information
  • CPU specifications (Apple M1 Max)

Key Assessment Points:

  1. PR Scope vs. Golden Changes Mismatch: The PR objectives focus on:

    • Bubble probe readiness
    • ICFL relaxation checks
    • Probe format fixes

    These are functional logic changes, not environmental configuration changes. Golden metadata files should only be regenerated when the test outputs (numerical results, probe data, etc.) change.

  2. Environmental Drift Risk: Updating golden files with machine-specific paths, compiler versions, and CPU specs creates problems:

    • Different CI environments will have different compiler versions
    • Test reproducibility suffers across machines
    • Environmental drift masks real functional regressions
  3. Learning Alignment: The retrieved learning directly supports the concern: "ensure no regressions in golden test outputs without clear justification"

Based on established testing practices and the learning context, the original review comment's concern is well-founded. Without access to the full PR diff and prior golden file state, I cannot definitively verify whether the functional probe changes actually warrant any golden file updates, but I can confirm that environmental-only changes should not trigger golden file regeneration.

Rewritten Review Comment:


Golden metadata should not be regenerated for environmental changes alone.

This golden metadata file captures build configuration at test time. The updates (CMake version, compiler paths, CPU specs, timestamps) reflect environmental differences, not functional algorithm changes. The PR's stated objectives—bubble probe readiness, ICFL checks, probe format fixes—are code-level changes that may affect numeric test outputs, but environmental configuration should not be committed to golden files.

Golden test baselines must remain stable across CI systems and developer machines. Regenerating golden metadata for environment-specific details (compiler versions, paths, machine CPU) complicates reproducibility and risks obscuring real functional regressions.

Action required:

  1. If the probe-related code changes produce different numeric test outputs (probe data, simulation results), document which changes caused them and regenerate only the affected output portions with clear justification.
  2. If only functional logic changed without affecting test outputs, revert this golden file to its prior state. Environment-specific metadata should not be committed.
  3. Review whether golden-metadata.txt should be excluded from version control or regenerated in a normalized, environment-independent manner.

tests/092A339C/golden-metadata.txt (1)

1-119: Since the repository clone failed, I cannot access the git history directly. However, I can work with the information already provided in the review context. Let me analyze what we know:

Analysis of the situation:

  1. Golden file purpose: The file tests/092A339C/golden-metadata.txt captures environment metadata for test reproducibility
  2. Content of changes: The file shows only environmental differences (macOS/Apple Silicon toolchain vs. prior system, compiler versions, machine-specific paths)
  3. PR objectives stated: "bubble probe readiness, ICFL checks, probe output format, and bubble probe initialization" — all functional concerns
  4. Learning guidance: "ensure no regressions in golden test outputs without clear justification"
  5. PR title ambiguity: "Probe goldenfiles?" suggests the author wasn't certain about including these changes

Key observation: The review comment correctly identifies that:

  • Changes are purely environmental metadata
  • No functional changes to probe code are visible in this file
  • Absolute paths (/Users/spencer/, /opt/homebrew/) are machine-specific and non-portable
  • There's a scope mismatch between what changed (environment) and PR objectives (probe functionality)

The review comment's concerns are valid and grounded. The core issue is: environmental metadata changes alone should not trigger golden file updates unless there are accompanying functional changes that require test output regeneration.


Rewritten Review Comment

Golden file should not be updated for environment-specific metadata changes without corresponding functional changes.

This golden metadata file contains only environmental differences (macOS/Apple Silicon toolchain, CMake version, machine-specific absolute paths) unrelated to the PR's functional objectives (bubble probe readiness, ICFL checks, probe output format, and bubble probe initialization). Per the learnings, golden test outputs should be updated only with clear justification—typically when functional code changes produce different outputs.

Actions needed:

  1. Verify intentionality: Confirm whether this golden file update was deliberate or collateral from testing on a different machine.
  2. Check for functional changes: If the PR includes actual probe-related functional changes, those should justify the golden file update. If not, this file should be reverted.
  3. Normalize paths: If golden metadata files must capture toolchain information, consider normalizing absolute paths (e.g., replacing /Users/spencer/Downloads/MFC with relative paths or environment variables) to improve portability across different build environments.
tests/0853FBD1/golden-metadata.txt (1)

1-119: I'll help you verify the review comment about the golden metadata file changes. Let me investigate the context and scope of the changes.
<function_calls>

First, understand the PR structure and find related files

git ls-files tests/0853FBD1/ | head -20


</function_calls>

<function_calls>

Check the git status and diff to see what changed in this test

git diff HEAD~1 tests/0853FBD1/ --stat 2>/dev/null || git diff --cached tests/0853FBD1/ --stat 2>/dev/null || echo "Checking git log for context..."


</function_calls>

<function_calls>

Check the PR description and commit messages for context

git log --oneline -20 | head -10


</function_calls>

<function_calls>

Get detailed status of this test directory

ls -la tests/0853FBD1/ 2>/dev/null || echo "Directory not found"


</function_calls>

<function_calls>

Check what golden files typically look like in this repo - find golden files

fd -e "golden" tests/ | head -20


</function_calls>

<function_calls>

Search for golden metadata files more broadly

fd "golden-metadata" tests/ 2>/dev/null | head -30


</function_calls>

<function_calls>

Look at the PR description and understand the context

Check the HEAD commit message

git log -1 --format="%B" 2>/dev/null | head -50


</function_calls>

<function_calls>

Look for documentation on the test case and golden files

cat tests/0853FBD1/golden-metadata.txt 2>/dev/null | head -30


</function_calls>

<function_calls>

Check if there's a README or docs about golden files and test generation

find . -maxdepth 3 -name "README*" -o -name "golden" | grep -E "(README|TESTING|golden)" | grep -v ".git" | head -20


</function_calls>

<function_calls>

Check the mfc.sh script mentioned in the golden metadata to understand test generation

fd "mfc.sh" | head -5


</function_calls>

<function_calls>

Look for any recent changes to the test regeneration

Check if there are notes about the --generate flag behavior

rg "golden-metadata" --type txt -C 2 | head -50


</function_calls>

<function_calls>

Check the PR branch and see if there are other golden files modified

git log --all --pretty=format:"%h %s" | head -20


</function_calls>

<function_calls>

Check if this is a test metadata file that captures environment at test creation

Look for patterns in how tests are structured

ls -la tests/ | head -20


</function_calls>

<function_calls>

Examine what other files are in the test case directory

cd tests/0853FBD1 && ls -la && cd - > /dev/null


</function_calls>

<function_calls>

Count lines and view structure of the golden file

wc -l tests/0853FBD1/golden-metadata.txt 2>/dev/null

Also check if there are other golden files with data

find tests/0853FBD1 -name "golden*" -exec wc -l {} ;


</function_calls>

<function_calls>

Check git history of this specific file to see if it's frequently regenerated

git log --oneline tests/0853FBD1/golden-metadata.txt 2>/dev/null | head -10


</function_calls>

<function_calls>

Check if there's documentation in the repo about test management and golden files

fd "CONTRIBUTING" . | head -5


</function_calls>

<function_calls>

Look at the actual PR diff to see what was changed

Since we're on a PR branch, check what files were modified

git diff --name-only HEAD1 HEAD 2>/dev/null | head -30 || echo "Cannot access HEAD1, trying current status..."


</function_calls>

<function_calls>

Use GitHub CLI to check PR details if available

gh pr view 2>/dev/null | head -50 || echo "gh pr view not available, checking local git..."


</function_calls>

Let me search for documentation about how golden files are managed in this repository:
<function_calls>

Search for references to "golden" in documentation or scripts

rg "golden" --type markdown --type txt -i | head -50


</function_calls>

<function_calls>

Look at what a golden-metadata file contains and understand its purpose

Check the first and last lines to understand structure

head -20 tests/0853FBD1/golden-metadata.txt && echo "..." && tail -20 tests/0853FBD1/golden-metadata.txt


</function_calls>

<function_calls>

Check if there are other test golden files to understand the pattern better

find tests -name "golden-metadata.txt" | wc -l
find tests -name "golden*.txt" | head -10


</function_calls>

<function_calls>

Check if golden files are in .gitignore (which would indicate they're regenerated)

cat .gitignore 2>/dev/null | grep -i golden
echo "---"

Also check if tests directory has any .gitignore

cat tests/.gitignore 2>/dev/null | head -20


</function_calls>

<function_calls>

Check the git status of this file to confirm it's tracked

git status tests/0853FBD1/golden-metadata.txt 2>/dev/null
echo "---"

Also check if it's in git

git ls-files tests/0853FBD1/golden-metadata.txt 2>/dev/null && echo "File is tracked in git" || echo "File is not tracked"


</function_calls>

<function_calls>

Get more context on the PR - check what functional changes are in other files

Look for changes related to bubble probe, ICFL, or format strings

git log --all --pretty=format:"%h %s" | grep -i "probe|icfl|format" | head -10


</function_calls>

<function_calls>

Check README or documentation in the root

ls -la | grep -i readme

Check if there's testing documentation

find . -maxdepth 2 -name "TEST" -o -name "test" | grep -v ".git" | head -10


</function_calls>

Let me check the actual structure of the repository more systematically:
<function_calls>

Get a high-level view of the repo

git remote -v 2>/dev/null
echo "---"
git branch -a 2>/dev/null | head -10


</function_calls>

<function_calls>

Check what commit hash the golden file references

grep "Git:" tests/0853FBD1/golden-metadata.txt
echo "---"

Check current HEAD

git rev-parse --short HEAD 2>/dev/null


</function_calls>

<function_calls>

Check the current branch and compare with the golden file reference

git branch --show-current 2>/dev/null
echo "---"
git describe --all --contains bd9b9c6 2>/dev/null || echo "Commit not found or different reference"


</function_calls>

Let me take a more direct approach and analyze the actual file content compared to what would be expected:
<function_calls>

Let's examine what changed in this specific file by looking at git history

First check if there's a previous version to compare

git log -p tests/0853FBD1/golden-metadata.txt 2>/dev/null | head -200


</function_calls>

<function_calls>

Check if there are any CI workflows that regenerate golden files

find . -path "./.git" -prune -o -name ".yml" -o -name ".yaml" | grep -E "(workflow|github|ci)" | head -10


</function_calls>

<function_calls>

Search for any build or test scripts that might handle golden files

fd -e "sh" -e "py" . | xargs rg "golden.*generate|generate.*golden" 2>/dev/null | head -20


</function_calls>

Based on my investigation, I need to gather more specific information about whether this golden file update is justified. Let me focus on understanding the test case purpose:
<function_calls>

Look for any README or documentation specific to test 0853FBD1

find tests/0853FBD1 -type f | xargs ls -lh
echo "---"

Check if there's a test case definition or configuration

cat tests/0853FBD1/input* 2>/dev/null | head -30


</function_calls>

Based on my investigation, I have gathered sufficient information to provide a rewritten review. The key findings are:

  1. The golden-metadata.txt file is a tracked file in git (part of the PR changes)
  2. It captures build environment metadata at test creation time
  3. The file contains machine-specific details (compiler versions, paths, CMake version, CPU info, timestamps)
  4. The original review comment raised valid concerns that are supported by repository best practices (per the learnings: "ensure no regressions in golden test outputs without clear justification")

The review comment's concern is justified because golden files should only be updated when there are corresponding functional test changes that warrant regeneration. The metadata update alone, while reflecting environmental differences, requires explicit justification in the PR context.

Golden metadata file update lacks clear justification in PR context.

This golden-metadata.txt file captures build environment details (compiler versions, paths, CMake configuration, timestamps) at test creation. While environment-specific details can change, golden files tracked in version control should be regenerated only when there are corresponding functional test output changes or explicit justification for the regeneration.

The PR objectives focus on bubble probe readiness, ICFL checks, and probe output formats. Before approving, verify:

  1. Whether the functional test outputs (beyond metadata) reflect expected changes from these code modifications
  2. Whether this test regeneration was intentional as part of the PR, or if it reflects an incidental environmental difference
  3. If your test update strategy intends to capture environment metadata, clarify why this differs from previous test runs
src/post_process/m_data_output.fpp (1)

756-765: Inconsistent API usage between LO_OFFSET and HI_OFFSET in 1D path.

In the 1D branch, DBOPT_LO_OFFSET uses the array variant DBADDIAOPT with count=1, but DBOPT_HI_OFFSET still uses the integer variant DBADDIOPT. Since both lo_offset and hi_offset are allocated as arrays (even in 1D, per lines 177-179), this asymmetry appears unintentional.

Apply this diff to make the 1D path consistent:

             else
                err = DBMKOPTLIST(2, optlist)
                err = DBADDIAOPT(optlist, DBOPT_LO_OFFSET, 1, lo_offset)
-               err = DBADDIOPT(optlist, DBOPT_HI_OFFSET, hi_offset)
+               err = DBADDIAOPT(optlist, DBOPT_HI_OFFSET, 1, hi_offset)
                err = DBPUTQM(dbfile, 'rectilinear_grid', 16, &
                              'x', 1, 'y', 1, 'z', 1, &
                              x_cb, DB_F77NULL, DB_F77NULL, dims, 1, &
                              DB_DOUBLE, DB_COLLINEAR, &
                              optlist, ierr)
                err = DBFREEOPTLIST(optlist)
             end if
♻️ Duplicate comments (9)
tests/0D21B728/golden-metadata.txt (1)

1-119: Same machine-specific metadata concerns as other golden files.

This file has the same issues: user-specific paths, "dirty" git state, and local machine metadata. See the comment on tests/09DDCC0C/golden-metadata.txt for details.

tests/0FF51EDE/golden-metadata.txt (1)

1-119: Same machine-specific metadata concerns as other golden files.

This file has the same issues: user-specific paths, "dirty" git state, and local machine metadata. See the comment on tests/09DDCC0C/golden-metadata.txt for details.

tests/07C1FECC/golden-metadata.txt (1)

1-120: Environment-specific golden metadata pattern repeated across multiple test files.

This file exhibits the same environment-specific metadata updates seen in tests/023DC5B9/golden-metadata.txt and other golden metadata files in this PR. The systematic replacement of compiler/toolchain/CPU metadata across multiple test golden files warrants a single consolidated justification rather than per-file updates.

tests/0347811A/golden-metadata.txt (1)

1-120: Environment-specific golden metadata updates require consolidated justification.

This file is part of the systematic golden file metadata updates across the PR. See tests/023DC5B9/golden-metadata.txt comment for consolidated feedback on justifying environment-specific golden updates.

tests/0A362971/golden-metadata.txt (1)

1-120: Environment-specific golden metadata updates require consolidated justification.

This file is part of the systematic golden file metadata updates across the PR. See tests/023DC5B9/golden-metadata.txt comment for consolidated feedback on justifying environment-specific golden updates.

tests/11DAFD23/golden-metadata.txt (1)

1-120: Environment-specific golden metadata updates require consolidated justification.

This file is part of the systematic golden file metadata updates across the PR. See tests/023DC5B9/golden-metadata.txt comment for consolidated feedback on justifying environment-specific golden updates.

tests/02748F0F/golden-metadata.txt (1)

1-120: Environment-specific golden metadata updates require consolidated justification.

This file is part of the systematic golden file metadata updates across the PR. See tests/023DC5B9/golden-metadata.txt comment for consolidated feedback on justifying environment-specific golden updates.

tests/0879E062/golden-metadata.txt (1)

1-120: Environment-specific golden metadata updates require consolidated justification.

This file is part of the systematic golden file metadata updates across the PR. See tests/023DC5B9/golden-metadata.txt comment for consolidated feedback on justifying environment-specific golden updates.

tests/0288BDAD/golden-metadata.txt (1)

1-120: Environment-specific golden metadata updates require consolidated justification.

This file is part of the systematic golden file metadata updates across the PR. See tests/023DC5B9/golden-metadata.txt comment for consolidated feedback on justifying environment-specific golden updates.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e2ce07d and 8e8d5fb.

📒 Files selected for processing (63)
  • src/post_process/m_data_output.fpp (3 hunks)
  • src/simulation/m_data_output.fpp (17 hunks)
  • src/simulation/m_derived_variables.fpp (1 hunks)
  • src/simulation/m_start_up.fpp (1 hunks)
  • src/simulation/m_time_steppers.fpp (2 hunks)
  • tests/0045D9F8/golden-metadata.txt (4 hunks)
  • tests/0083C150/golden-metadata.txt (4 hunks)
  • tests/0083C150/golden.txt (1 hunks)
  • tests/016C1B8B/golden-metadata.txt (3 hunks)
  • tests/023DC5B9/golden-metadata.txt (5 hunks)
  • tests/02748F0F/golden-metadata.txt (4 hunks)
  • tests/02748F0F/golden.txt (1 hunks)
  • tests/0288BDAD/golden-metadata.txt (3 hunks)
  • tests/0288BDAD/golden.txt (1 hunks)
  • tests/02B08B80/golden-metadata.txt (4 hunks)
  • tests/02B08B80/golden.txt (1 hunks)
  • tests/02D1E522/golden-metadata.txt (4 hunks)
  • tests/02D1E522/golden.txt (1 hunks)
  • tests/0347811A/golden-metadata.txt (9 hunks)
  • tests/0347811A/golden.txt (1 hunks)
  • tests/0387594F/golden-metadata.txt (2 hunks)
  • tests/03D598D0/golden-metadata.txt (5 hunks)
  • tests/043B535A/golden-metadata.txt (4 hunks)
  • tests/04E1CB26/golden-metadata.txt (9 hunks)
  • tests/0501B3DA/golden-metadata.txt (5 hunks)
  • tests/0501B3DA/golden.txt (1 hunks)
  • tests/059D137F/golden-metadata.txt (2 hunks)
  • tests/07C1FECC/golden-metadata.txt (4 hunks)
  • tests/07C33719/golden-metadata.txt (5 hunks)
  • tests/07C54EDD/golden-metadata.txt (5 hunks)
  • tests/07D9EBD1/golden-metadata.txt (7 hunks)
  • tests/0853FBD1/golden-metadata.txt (6 hunks)
  • tests/0879E062/golden-metadata.txt (4 hunks)
  • tests/0879E062/golden.txt (1 hunks)
  • tests/092A339C/golden-metadata.txt (9 hunks)
  • tests/09623DE3/golden-metadata.txt (5 hunks)
  • tests/09DAFEBA/golden-metadata.txt (4 hunks)
  • tests/09DDCC0C/golden-metadata.txt (3 hunks)
  • tests/0A022883/golden-metadata.txt (4 hunks)
  • tests/0A362971/golden-metadata.txt (5 hunks)
  • tests/0A62F0A6/golden-metadata.txt (6 hunks)
  • tests/0A62F0A6/golden.txt (1 hunks)
  • tests/0ACB1F16/golden-metadata.txt (4 hunks)
  • tests/0B07F3A1/golden-metadata.txt (6 hunks)
  • tests/0B07F3A1/golden.txt (1 hunks)
  • tests/0BAA2F42/golden-metadata.txt (6 hunks)
  • tests/0BD42F35/golden-metadata.txt (4 hunks)
  • tests/0BDC8E40/golden-metadata.txt (7 hunks)
  • tests/0D1FA5C5/golden-metadata.txt (3 hunks)
  • tests/0D21B728/golden-metadata.txt (3 hunks)
  • tests/0F6DB678/golden-metadata.txt (8 hunks)
  • tests/0FCCE9F1/golden-metadata.txt (4 hunks)
  • tests/0FCCE9F1/golden.txt (1 hunks)
  • tests/0FF51EDE/golden-metadata.txt (1 hunks)
  • tests/0FF51EDE/golden.txt (1 hunks)
  • tests/10041BB7/golden-metadata.txt (4 hunks)
  • tests/105673CA/golden-metadata.txt (8 hunks)
  • tests/106C0BE6/golden-metadata.txt (4 hunks)
  • tests/10DBA76B/golden-metadata.txt (4 hunks)
  • tests/10DE58AA/golden-metadata.txt (1 hunks)
  • tests/11DAFD23/golden-metadata.txt (8 hunks)
  • tests/11DAFD23/golden.txt (2 hunks)
  • tests/121D4ECA/golden-metadata.txt (4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{fpp,f90}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{fpp,f90}: Use 2-space indentation; continuation lines align beneath &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_ pattern (e.g., m_transport)
Name public subroutines with s_ pattern (e.g., s_compute_flux)
Name public functions with f
_ pattern
Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Limit routine arguments to ≤ 6; use derived-type params struct if more are needed
Forbid goto statements (except in legacy code), COMMON blocks, and save globals
Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Call s_mpi_abort() for errors, never use stop or error stop

**/*.{fpp,f90}: Indent 2 spaces; continuation lines align under &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_<feature> prefix (e.g., m_transport)
Name public subroutines as s_<verb>_<noun> (e.g., s_compute_flux) and functions as f_<verb>_<noun>
Keep private helpers in the module; avoid nested procedures
Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct
Avoid goto statements (except unavoidable legacy); avoid global state (COMMON, save)
Every variable must have intent(in|out|inout) specification and appropriate dimension / allocatable / pointer
Use s_mpi_abort(<msg>) for error termination instead of stop
Use !> style documentation for header comments; follow Doxygen Fortran format with !! @param and !! @return tags
Use implicit none statement in all modules
Use private declaration followed by explicit public exports in modules
Use derived types with pointers for encapsulation (e.g., pointer, dimension(:,:,:) => null())
Use pure and elemental attributes for side-effect-free functions; combine them for array ...

Files:

  • src/simulation/m_derived_variables.fpp
  • src/simulation/m_time_steppers.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_start_up.fpp
  • src/post_process/m_data_output.fpp
src/simulation/**/*.{fpp,f90}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/simulation/**/*.{fpp,f90}: Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Avoid stop/error stop inside GPU device code
Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers

src/simulation/**/*.{fpp,f90}: Mark GPU-callable helpers with $:GPU_ROUTINE(function_name='...', parallelism='[seq]') immediately after declaration
Do not use OpenACC or OpenMP directives directly; use Fypp macros from src/common/include/parallel_macros.fpp instead
Wrap tight loops with $:GPU_PARALLEL_FOR(private='[...]', copy='[...]') macro; add collapse=n for safe nested loop merging
Declare loop-local variables with private='[...]' in GPU parallel loop macros
Allocate large arrays with managed or move them into a persistent $:GPU_ENTER_DATA(...) region at start-up
Do not place stop or error stop inside device code

Files:

  • src/simulation/m_derived_variables.fpp
  • src/simulation/m_time_steppers.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_start_up.fpp
src/**/*.fpp

📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)

src/**/*.fpp: Use .fpp file extension for Fypp preprocessed files; CMake transpiles them to .f90
Start module files with Fypp include for macros: #:include 'macros.fpp'
Use the fypp ASSERT macro for validating conditions: @:ASSERT(predicate, message)
Use fypp macro @:ALLOCATE(var1, var2) for device-aware allocation instead of standard Fortran allocate
Use fypp macro @:DEALLOCATE(var1, var2) for device-aware deallocation instead of standard Fortran deallocate

Files:

  • src/simulation/m_derived_variables.fpp
  • src/simulation/m_time_steppers.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_start_up.fpp
  • src/post_process/m_data_output.fpp
🧠 Learnings (21)
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Add and update focused tests for changes; ensure no regressions in golden test outputs without clear justification

Applied to files:

  • tests/0FCCE9F1/golden.txt
  • tests/043B535A/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0A62F0A6/golden.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/0387594F/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/121D4ECA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/0501B3DA/golden.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/10DE58AA/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0083C150/golden.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
  • tests/0FF51EDE/golden.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0879E062/golden.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/02B08B80/golden.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/0B07F3A1/golden.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/0347811A/golden.txt
  • tests/0288BDAD/golden.txt
  • tests/11DAFD23/golden.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/02D1E522/golden.txt
  • tests/02748F0F/golden.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/0387594F/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/121D4ECA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • src/simulation/m_derived_variables.fpp
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/10DE58AA/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • src/simulation/m_start_up.fpp
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/0387594F/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/121D4ECA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/10DE58AA/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/121D4ECA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • src/simulation/m_data_output.fpp
  • tests/10041BB7/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • src/simulation/m_start_up.fpp
  • tests/016C1B8B/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/0387594F/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/121D4ECA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • src/simulation/m_derived_variables.fpp
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • src/simulation/m_data_output.fpp
  • tests/10041BB7/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • src/simulation/m_start_up.fpp
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Compile with Cray `ftn` or NVIDIA `nvfortran` for GPU offloading; also build CPU-only with GNU `gfortran` and Intel `ifx`/`ifort` for portability

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/121D4ECA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • src/simulation/m_derived_variables.fpp
  • tests/09623DE3/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • src/simulation/m_data_output.fpp
  • tests/07D9EBD1/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • src/simulation/m_start_up.fpp
  • tests/016C1B8B/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: After each successful build step, run focused tests using `./mfc.sh test -j $(nproc) -f EA8FA07E -t 9E2CA336` instead of running all tests

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/0387594F/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/121D4ECA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/**/*.fpp : Use `.fpp` file extension for Fypp preprocessed files; CMake transpiles them to `.f90`

Applied to files:

  • tests/043B535A/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Review Fypp macros in src/<subprogram>/include/ before reviewing generated code

Applied to files:

  • tests/0B07F3A1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Fypp macros are defined in `src/<subprogram>/include/` directories where `<subprogram>` ∈ {`simulation`,`common`,`pre_process`,`post_process`}; scan these first when understanding macro usage

Applied to files:

  • tests/0B07F3A1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/**/*.fpp : Use fypp macro `@:ALLOCATE(var1, var2)` for device-aware allocation instead of standard Fortran `allocate`

Applied to files:

  • tests/0B07F3A1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/**/*.fpp : Start module files with Fypp include for macros: `#:include 'macros.fpp'`

Applied to files:

  • tests/07C33719/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Avoid stop/error stop inside GPU device code

Applied to files:

  • src/simulation/m_derived_variables.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_start_up.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Mark GPU-callable helpers with `$:GPU_ROUTINE(function_name='...', parallelism='[seq]')` immediately after declaration

Applied to files:

  • src/simulation/m_time_steppers.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_start_up.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large arrays with `managed` or move them into a persistent `$:GPU_ENTER_DATA(...)` region at start-up

Applied to files:

  • tests/09DDCC0C/golden-metadata.txt
  • src/simulation/m_data_output.fpp
  • src/simulation/m_start_up.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to **/*.{fpp,f90} : Use `private` declaration followed by explicit `public` exports in modules

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros

Applied to files:

  • src/simulation/m_data_output.fpp
  • src/simulation/m_start_up.fpp
📚 Learning: 2025-11-24T21:50:46.879Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.879Z
Learning: Applies to **/*.{fpp,f90} : Use `wp` (working precision) parameter from `m_precision_select` instead of hardcoded precision like `real*8`

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up

Applied to files:

  • src/simulation/m_start_up.fpp
📚 Learning: 2025-11-24T21:50:16.684Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.684Z
Learning: Applies to **/*.{fpp,f90} : Name public subroutines with s_<verb>_<noun> pattern (e.g., s_compute_flux)

Applied to files:

  • src/simulation/m_start_up.fpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Self Hosted (cpu, none, gt)
  • GitHub Check: Self Hosted (cpu, none, frontier)
  • GitHub Check: Self Hosted (gpu, acc, gt)
  • GitHub Check: Self Hosted (gpu, omp, gt)
  • GitHub Check: Self Hosted (gpu, omp, frontier)
  • GitHub Check: Self Hosted (gpu, acc, frontier)
  • GitHub Check: Georgia Tech | Phoenix (NVHPC) (cpu)

Comment on lines +1657 to 1667
write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8)') &
nondim_time, &
rho, &
vel(1), &
pres
print *, 'time =', nondim_time, 'rho =', rho, 'pres =', pres
vel(2), &
pres, &
gamma, &
pi_inf, &
qv, &
c
end if
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Format string has one extra specifier for 2D case.

The format string has 10 specifiers (1 F12.6 + 9 F24.8) but only 9 values are written. The extra F24.8 won't cause a runtime error but is inconsistent with the 3D case which correctly matches 10 specifiers to 10 values.

-                        write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8)') &
+                        write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8)') &
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8)') &
nondim_time, &
rho, &
vel(1), &
pres
print *, 'time =', nondim_time, 'rho =', rho, 'pres =', pres
vel(2), &
pres, &
gamma, &
pi_inf, &
qv, &
c
end if
write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8)') &
nondim_time, &
rho, &
vel(1), &
vel(2), &
pres, &
gamma, &
pi_inf, &
qv, &
c
end if

Comment on lines 1 to 7
This file was created on 2025-11-29 14:50:17.662829.

mfc.sh:

Invocation: test --generate -o 106C0BE6
Lock: mpi=Yes & gpu=No & debug=No & gcov=No & unified=No & single=No
Git: 63ce00e8e87c688511c03c18b89c4883db98da74 on FrontierUVM (dirty)
Invocation: test -j 10 --generate
Lock: mpi=Yes & gpu=No & debug=No & gcov=No & unified=No & single=No & mixed=No & fastmath=No
Git: bd9b9c6a04a916420687d762e135106d9c59e700 on probe-testing (dirty)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's see all changes in this PR related to golden files
git diff --name-only HEAD~1 HEAD | grep -i golden

Repository: MFlowCode/MFC

Length of output: 170


🏁 Script executed:

# Let's check the PR description and objectives more clearly
gh pr view 0 --json body,title -q '.title + "\n\n" + .body' 2>/dev/null || echo "PR metadata not available"

Repository: MFlowCode/MFC

Length of output: 170


🏁 Script executed:

# Look for test-related changes that might require golden regeneration
git diff --stat HEAD~1 HEAD | head -30

Repository: MFlowCode/MFC

Length of output: 170


🏁 Script executed:

# Check for changes to probe-related code (bubble probe, ICFL)
git diff HEAD~1 HEAD --name-only | grep -E "(probe|bubble|icfl)" -i

Repository: MFlowCode/MFC

Length of output: 170


🏁 Script executed:

[skip_cloning]
# Check if there's public documentation about this PR or issue
curl -s "https://api.github.com/repos/MFlowCode/MFC/pulls/0" | python3 << 'EOF'
import json
import sys
data = json.load(sys.stdin)
print("PR Title:", data.get('title', 'N/A'))
print("PR State:", data.get('state', 'N/A'))
print("Created:", data.get('created_at', 'N/A'))
print("Modified:", data.get('updated_at', 'N/A'))
print("\nBody:")
print(data.get('body', 'N/A')[:1000])
EOF

Repository: MFlowCode/MFC

Length of output: 170


Based on my analysis using the provided context and the project learnings, I can now provide the rewritten review comment:


Golden metadata regenerated on different machine—clarify necessity.

Golden test outputs should not change without clear justification, per project learnings. This metadata file shows environment-specific regeneration (timestamp, compiler versions, machine hostname, CPU details), indicating the test was re-run on a different system (Apple M1 Max) rather than the original build environment.

The PR objectives focus on functional improvements (bubble probe readiness, ICFL relaxation, probe output fixes), not environment transitions. If these functional changes legitimately necessitate golden file regeneration, this should be documented explicitly in the PR.

Request clarification: Are the metadata changes necessary to support the functional changes, or are they incidental environment artifacts from local testing? If intentional, document why the baseline needed regeneration.

🤖 Prompt for AI Agents
In tests/106C0BE6/golden-metadata.txt around lines 1 to 7, the golden metadata
was regenerated with environment-specific data (timestamp, machine, CPU, Git
dirty state) which appears incidental to this PR; ensure golden files are only
updated when functionally required: either revert this file to the original
committed metadata if the functional changes do not require regeneration, or
update the PR description and commit message to explicitly state why the golden
metadata must be refreshed (include which functional change necessitates it and
confirm reproducibility steps/environment), and remove transient details like
timestamps/hostnames from the golden file if they are not essential.

Comment on lines +13 to +16
CMake v4.1.2 on shb-m1pro-3.local

C : AppleClang v17.0.0.17000013 (/Library/Developer/CommandLineTools/usr/bin/cc)
Fortran : GNU v14.2.0 (/opt/homebrew/bin/gfortran)
C : AppleClang v17.0.0.17000404 (/usr/bin/cc)
Fortran : GNU v15.2.0 (/opt/homebrew/bin/gfortran)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

❓ Verification inconclusive

🏁 Script executed:

# First, let's read the golden metadata file to see the actual content
cat -n tests/106C0BE6/golden-metadata.txt | head -80

Repository: MFlowCode/MFC

Length of output: 170


🏁 Script executed:

# Check git history to see if this file was modified in this PR
git diff HEAD~1 tests/106C0BE6/golden-metadata.txt 2>/dev/null || git diff tests/106C0BE6/golden-metadata.txt 2>/dev/null || echo "File may not be in tracked changes"

Repository: MFlowCode/MFC

Length of output: 170


🏁 Script executed:

# Search for any sanitization or path replacement logic in the codebase
rg -n "sanitiz|golden|metadata" --type=py --type=sh --type=cmake -i 2>/dev/null | head -50

Repository: MFlowCode/MFC

Length of output: 170


🏁 Script executed:

# Look for any documentation or guidelines on golden tests
fd -e md -e txt -e rst | xargs rg -l "golden" 2>/dev/null | head -10

Repository: MFlowCode/MFC

Length of output: 170


I'm unable to clone the repository to directly verify the content, but I can analyze the review comment based on the context provided and software engineering best practices.

Based on the review comment itself, I can assess the validity of the concern:

Key observations:

  1. The review identifies specific machine-specific paths: /Users/spencer/Downloads/MFC/build/venv/bin/fypp (user-specific absolute path)
  2. Compiler versions and tools are cited as environment-specific (CMake v4.1.2, GNU Fortran v15.2.0, AppleClang v17.0.0)
  3. The concern is flagged as with
  4. The issue affects multiple lines (13-16, 27, 29, 36-38, 47, 49-50, 61, 63, 70-72)

Analysis:

  • Golden metadata files in test suites typically capture expected output from test runs
  • Absolute user-specific paths like /Users/spencer/Downloads/... are problematic for reproducibility across different systems and CI environments
  • However, compiler versions may be environment-specific by design (tests may document what versions were used to generate baseline)
  • The distinction matters: paths should be sanitized, but version information may be intentional

Without access to the codebase to verify:

  • Whether this PR introduced or modified these paths
  • The project's explicit policy on golden test reproducibility
  • Whether there are existing sanitization mechanisms
  • The PR context and rationale for changes to this file

I cannot definitively confirm or refute the concern with certainty.


Compiler and environment paths in golden metadata should be verified for reproducibility.

The review identifies absolute user-specific paths (e.g., /Users/spencer/Downloads/MFC/build/venv/bin/fypp) and environment-specific compiler versions in the golden metadata file at lines 13-16, 27, 29, 36-38, 47, 49-50, 61, 63, and 70-72. These details warrant review to determine:

  • Whether this PR intentionally modifies the golden baseline and if so, whether paths can be sanitized (e.g., relative to build directory or a placeholder)
  • Whether the project has established conventions for environment-agnostic golden test outputs

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 378 to 385
! Check for problematic ICFL values
! For bubble cases: allow both NaN and Infinity (can occur during startup/transients)
! For non-bubble cases: NaN is always bad, Infinity might be bad
if (.not. f_approx_equal(icfl_max_glb, icfl_max_glb) .and. .not. bubbles_euler) then
! ICFL is NaN in non-bubble case - this is always bad
call s_mpi_abort('ICFL is NaN. Exiting.')
elseif (icfl_max_glb > 1._wp) then
elseif (icfl_max_glb > 1._wp .and. icfl_max_glb < huge(1._wp) .and. .not. bubbles_euler) then
! ICFL > 1.0 but finite in non-bubble case - stability problem

Choose a reason for hiding this comment

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

P1 Badge ICFL safety checks bypassed for all bubble simulations

The new guard around the ICFL sanity checks if (.not. bubbles_euler) means bubble_euler runs will never abort on NaN or ICFL > 1, not just during the startup window the commit message refers to. Once bubble probes are marked ready and ICFL starts being computed, any instability or overflow in bubble cases will silently continue instead of aborting, which masks unstable time steps and can corrupt results. Consider re-enabling the checks after initialization (or when bubble_probe_ready is true) so post-startup bubble runs still enforce ICFL validity. (See also lines ~381-385.)

Useful? React with 👍 / 👎.

Comment on lines +1223 to +1226
! minloc returns 1-based index into distx array
! Convert: 1-based minloc -> distx index (-1 to m) -> q_cons_vf index (0 to m)
j = minloc(distx, 1) - 2 ! -2 converts 1-based to distx index
if (j < 0) j = 0 ! Clamp to valid q_cons_vf range

Choose a reason for hiding this comment

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

P1 Badge Bubble probe now indexes two cells left of the intended location

With the probe index now converted to the actual field index via j = minloc(distx,1) - 2 (clamped at 0), j already matches the grid coordinates. The num_fluids==4 bubble branch later still dereferences q_cons_vf(...%sf(j - 2,0,0)), so j is shifted left by two cells (often negative at the boundary), producing out-of-bounds or incorrect probe output for 1D four-fluid bubble cases. Align the downstream accesses to use j directly (as done elsewhere in this routine) after this zero-basing change. The regression is triggered in bubble_euler 1D runs with four fluids.

Useful? React with 👍 / 👎.

@codeant-ai
Copy link

codeant-ai bot commented Nov 30, 2025

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (18)
tests/0083C150/golden-metadata.txt (1)

1-1: Same metadata regeneration concern as other test files.

This file exhibits the same environment-specific regeneration pattern seen across multiple golden-metadata.txt files in this PR (Apple Silicon, user-specific paths, timestamp updates).

Also applies to: 5-7, 13-16, 29-29, 38-38, 63-63, 97-97, 114-119

tests/043B535A/golden-metadata.txt (1)

1-1: Same metadata regeneration concern.

Also applies to: 5-7, 13-16, 29-29, 38-39, 63-63, 97-97, 113-119

tests/092A339C/golden-metadata.txt (1)

1-1: Same metadata regeneration concern.

Also applies to: 5-7, 13-16, 29-29, 38-38, 63-63, 97-97, 114-119

tests/106C0BE6/golden-metadata.txt (1)

1-1: Same metadata regeneration concern (previously flagged).

Past reviews already identified this pattern. The environment-specific regeneration across all golden-metadata.txt files suggests a systematic rebaseline on Apple Silicon rather than targeted functional test updates.

Also applies to: 5-7, 13-16, 29-29, 36-38, 63-63, 97-97, 115-119

tests/0F6DB678/golden-metadata.txt (1)

1-1: Same metadata regeneration concern.

Also applies to: 5-7, 13-16, 29-29, 38-38, 63-63, 97-97, 114-119

tests/10041BB7/golden-metadata.txt (1)

1-120: ****
Refer to review comment for tests/07D9EBD1/golden-metadata.txt (lines 1–120).

tests/016C1B8B/golden-metadata.txt (1)

1-120: ****
Refer to review comment for tests/07D9EBD1/golden-metadata.txt (lines 1–120).

tests/0045D9F8/golden-metadata.txt (1)

1-120: ****
Refer to review comment for tests/07D9EBD1/golden-metadata.txt (lines 1–120).

tests/09DDCC0C/golden-metadata.txt (1)

1-120: ****
Refer to review comment for tests/07D9EBD1/golden-metadata.txt (lines 1–120).

tests/07C1FECC/golden-metadata.txt (1)

1-120: ****
Refer to review comment for tests/07D9EBD1/golden-metadata.txt (lines 1–120).

tests/0347811A/golden-metadata.txt (1)

1-120: ****
Refer to review comment for tests/07D9EBD1/golden-metadata.txt (lines 1–120).

tests/02B08B80/golden-metadata.txt (1)

1-120: ****
Refer to review comment for tests/07D9EBD1/golden-metadata.txt (lines 1–120).

tests/105673CA/golden-metadata.txt (1)

1-154: Duplicate issue: Golden metadata generated on local developer environment.

Same concern as tests/10DBA76B/golden-metadata.txt - this golden file contains developer-specific paths and local machine identifiers, reducing reproducibility.

tests/0853FBD1/golden-metadata.txt (1)

1-120: Duplicate issue: Golden metadata generated on local developer environment.

Same reproducibility concern as identified in tests/10DBA76B/golden-metadata.txt.

tests/10DE58AA/golden-metadata.txt (1)

1-120: Duplicate issue: Golden metadata generated on local developer environment.

Same reproducibility concern as identified in tests/10DBA76B/golden-metadata.txt.

tests/0BAA2F42/golden-metadata.txt (1)

1-120: Duplicate issue: Golden metadata generated on local developer environment.

Same reproducibility concern as identified in tests/10DBA76B/golden-metadata.txt.

tests/0BD42F35/golden-metadata.txt (1)

1-120: Duplicate issue: Golden metadata generated on local developer environment.

Same reproducibility concern as identified in tests/10DBA76B/golden-metadata.txt.

src/simulation/m_data_output.fpp (1)

1670-1680: Format string mismatch: 10 specifiers for 9 values in 2D non-bubble case.

The format string has 10 F-specifiers but only 9 values are written. While Fortran may not error, this is inconsistent with the 3D case (line 1682-1693) which correctly matches 10 specifiers to 10 values.

Apply the fix from the previous review:

-                        write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8)') &
+                        write (i + 30, '(6X,F12.6,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8,F24.8)') &
🧹 Nitpick comments (2)
compare_goldenfiles.py (2)

42-53: Hardcoded branch name limits utility for comparing different branches.

The URL construction hardcodes the master branch. Consider accepting a --branch argument to enable comparisons against feature branches or tags.

 def fetch_repo_golden_file(test_uuid):
     """Fetch a golden file from GitHub raw content."""
-    url = f"https://raw.githubusercontent.com/mflowcode/mfc/master/tests/{test_uuid}/golden.txt"
+    branch = os.environ.get('MFC_COMPARE_BRANCH', 'master')
+    url = f"https://raw.githubusercontent.com/mflowcode/mfc/{branch}/tests/{test_uuid}/golden.txt"

Alternatively, add a --branch CLI argument in the main block.


200-200: Remove unnecessary f-string prefix.

Line 200 uses an f-string without placeholders.

-    print(f"\nSummary:")
+    print("\nSummary:")
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e8d5fb and 9ee9be0.

📒 Files selected for processing (50)
  • compare_goldenfiles.py (1 hunks)
  • golden_comparison_results.txt (1 hunks)
  • src/common/m_variables_conversion.fpp (2 hunks)
  • src/simulation/m_data_output.fpp (18 hunks)
  • tests/0045D9F8/golden-metadata.txt (7 hunks)
  • tests/0083C150/golden-metadata.txt (9 hunks)
  • tests/016C1B8B/golden-metadata.txt (6 hunks)
  • tests/023DC5B9/golden-metadata.txt (4 hunks)
  • tests/02748F0F/golden-metadata.txt (1 hunks)
  • tests/0288BDAD/golden-metadata.txt (4 hunks)
  • tests/02B08B80/golden-metadata.txt (9 hunks)
  • tests/02D1E522/golden-metadata.txt (9 hunks)
  • tests/0347811A/golden-metadata.txt (4 hunks)
  • tests/0387594F/golden-metadata.txt (7 hunks)
  • tests/03D598D0/golden-metadata.txt (6 hunks)
  • tests/043B535A/golden-metadata.txt (1 hunks)
  • tests/04E1CB26/golden-metadata.txt (7 hunks)
  • tests/0501B3DA/golden-metadata.txt (5 hunks)
  • tests/059D137F/golden-metadata.txt (3 hunks)
  • tests/07C1FECC/golden-metadata.txt (3 hunks)
  • tests/07C33719/golden-metadata.txt (4 hunks)
  • tests/07C54EDD/golden-metadata.txt (5 hunks)
  • tests/07D9EBD1/golden-metadata.txt (3 hunks)
  • tests/0853FBD1/golden-metadata.txt (4 hunks)
  • tests/0879E062/golden-metadata.txt (1 hunks)
  • tests/092A339C/golden-metadata.txt (7 hunks)
  • tests/09623DE3/golden-metadata.txt (4 hunks)
  • tests/09DAFEBA/golden-metadata.txt (1 hunks)
  • tests/09DDCC0C/golden-metadata.txt (4 hunks)
  • tests/0A022883/golden-metadata.txt (1 hunks)
  • tests/0A362971/golden-metadata.txt (6 hunks)
  • tests/0A62F0A6/golden-metadata.txt (5 hunks)
  • tests/0ACB1F16/golden-metadata.txt (8 hunks)
  • tests/0B07F3A1/golden-metadata.txt (8 hunks)
  • tests/0BAA2F42/golden-metadata.txt (4 hunks)
  • tests/0BD42F35/golden-metadata.txt (5 hunks)
  • tests/0BDC8E40/golden-metadata.txt (3 hunks)
  • tests/0D1FA5C5/golden-metadata.txt (6 hunks)
  • tests/0D21B728/golden-metadata.txt (2 hunks)
  • tests/0F6DB678/golden-metadata.txt (9 hunks)
  • tests/0FCCE9F1/golden-metadata.txt (1 hunks)
  • tests/0FF51EDE/golden-metadata.txt (5 hunks)
  • tests/10041BB7/golden-metadata.txt (3 hunks)
  • tests/105673CA/golden-metadata.txt (6 hunks)
  • tests/106C0BE6/golden-metadata.txt (3 hunks)
  • tests/10DBA76B/golden-metadata.txt (4 hunks)
  • tests/10DE58AA/golden-metadata.txt (3 hunks)
  • tests/11DAFD23/golden-metadata.txt (6 hunks)
  • tests/11DAFD23/golden.txt (2 hunks)
  • tests/121D4ECA/golden-metadata.txt (3 hunks)
✅ Files skipped from review due to trivial changes (1)
  • golden_comparison_results.txt
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/121D4ECA/golden-metadata.txt
  • tests/09DAFEBA/golden-metadata.txt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{fpp,f90}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{fpp,f90}: Use 2-space indentation; continuation lines align beneath &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_ pattern (e.g., m_transport)
Name public subroutines with s_ pattern (e.g., s_compute_flux)
Name public functions with f
_ pattern
Keep subroutine size ≤ 500 lines, helper subroutines ≤ 150 lines, functions ≤ 100 lines, files ≤ 1000 lines
Limit routine arguments to ≤ 6; use derived-type params struct if more are needed
Forbid goto statements (except in legacy code), COMMON blocks, and save globals
Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate
Call s_mpi_abort() for errors, never use stop or error stop

**/*.{fpp,f90}: Indent 2 spaces; continuation lines align under &
Use lower-case keywords and intrinsics (do, end subroutine, etc.)
Name modules with m_<feature> prefix (e.g., m_transport)
Name public subroutines as s_<verb>_<noun> (e.g., s_compute_flux) and functions as f_<verb>_<noun>
Keep private helpers in the module; avoid nested procedures
Enforce size limits: subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, module/file ≤ 1000
Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct
Avoid goto statements (except unavoidable legacy); avoid global state (COMMON, save)
Every variable must have intent(in|out|inout) specification and appropriate dimension / allocatable / pointer
Use s_mpi_abort(<msg>) for error termination instead of stop
Use !> style documentation for header comments; follow Doxygen Fortran format with !! @param and !! @return tags
Use implicit none statement in all modules
Use private declaration followed by explicit public exports in modules
Use derived types with pointers for encapsulation (e.g., pointer, dimension(:,:,:) => null())
Use pure and elemental attributes for side-effect-free functions; combine them for array ...

Files:

  • src/simulation/m_data_output.fpp
  • src/common/m_variables_conversion.fpp
src/simulation/**/*.{fpp,f90}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/simulation/**/*.{fpp,f90}: Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)
Allocate large GPU arrays with managed memory or move them into persistent !$acc enter data regions at start-up
Avoid stop/error stop inside GPU device code
Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers

src/simulation/**/*.{fpp,f90}: Mark GPU-callable helpers with $:GPU_ROUTINE(function_name='...', parallelism='[seq]') immediately after declaration
Do not use OpenACC or OpenMP directives directly; use Fypp macros from src/common/include/parallel_macros.fpp instead
Wrap tight loops with $:GPU_PARALLEL_FOR(private='[...]', copy='[...]') macro; add collapse=n for safe nested loop merging
Declare loop-local variables with private='[...]' in GPU parallel loop macros
Allocate large arrays with managed or move them into a persistent $:GPU_ENTER_DATA(...) region at start-up
Do not place stop or error stop inside device code

Files:

  • src/simulation/m_data_output.fpp
src/**/*.fpp

📄 CodeRabbit inference engine (.cursor/rules/mfc-agent-rules.mdc)

src/**/*.fpp: Use .fpp file extension for Fypp preprocessed files; CMake transpiles them to .f90
Start module files with Fypp include for macros: #:include 'macros.fpp'
Use the fypp ASSERT macro for validating conditions: @:ASSERT(predicate, message)
Use fypp macro @:ALLOCATE(var1, var2) for device-aware allocation instead of standard Fortran allocate
Use fypp macro @:DEALLOCATE(var1, var2) for device-aware deallocation instead of standard Fortran deallocate

Files:

  • src/simulation/m_data_output.fpp
  • src/common/m_variables_conversion.fpp
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Add and update focused tests for changes; ensure no regressions in golden test outputs without clear justification
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Add and update focused tests for changes; ensure no regressions in golden test outputs without clear justification

Applied to files:

  • tests/0387594F/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/10DE58AA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/11DAFD23/golden.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
  • compare_goldenfiles.py
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers

Applied to files:

  • tests/0387594F/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/10DE58AA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Draft a step-by-step plan before making changes; build after each step using `./mfc.sh build -t pre_process simulation -j $(nproc)`

Applied to files:

  • tests/0387594F/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/10DE58AA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead

Applied to files:

  • tests/0387594F/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • src/simulation/m_data_output.fpp
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Compile with Cray `ftn` or NVIDIA `nvfortran` for GPU offloading; also build CPU-only with GNU `gfortran` and Intel `ifx`/`ifort` for portability

Applied to files:

  • tests/0387594F/golden-metadata.txt
  • tests/02748F0F/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/0BDC8E40/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/07C54EDD/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/0D21B728/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0501B3DA/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/**/*.fpp : Use `.fpp` file extension for Fypp preprocessed files; CMake transpiles them to `.f90`

Applied to files:

  • tests/0387594F/golden-metadata.txt
  • tests/0083C150/golden-metadata.txt
  • tests/0BD42F35/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/0288BDAD/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0347811A/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging

Applied to files:

  • tests/02748F0F/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/016C1B8B/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/0D1FA5C5/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: After each successful build step, run focused tests using `./mfc.sh test -j $(nproc) -f EA8FA07E -t 9E2CA336` instead of running all tests

Applied to files:

  • tests/02748F0F/golden-metadata.txt
  • tests/0A62F0A6/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/10041BB7/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/023DC5B9/golden-metadata.txt
  • tests/07C1FECC/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/10DE58AA/golden-metadata.txt
  • tests/106C0BE6/golden-metadata.txt
  • tests/0BAA2F42/golden-metadata.txt
  • tests/09DDCC0C/golden-metadata.txt
  • tests/10DBA76B/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/0045D9F8/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • tests/059D137F/golden-metadata.txt
  • tests/0FF51EDE/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/07D9EBD1/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Wrap tight GPU loops with !$acc parallel loop gang vector default(present) reduction(...); add collapse(n) when safe; declare loop-local variables with private(...)

Applied to files:

  • tests/02748F0F/golden-metadata.txt
  • tests/043B535A/golden-metadata.txt
  • tests/0879E062/golden-metadata.txt
  • tests/0FCCE9F1/golden-metadata.txt
  • tests/0A022883/golden-metadata.txt
  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Review Fypp macros in src/<subprogram>/include/ before reviewing generated code

Applied to files:

  • tests/0083C150/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Fypp macros are defined in `src/<subprogram>/include/` directories where `<subprogram>` ∈ {`simulation`,`common`,`pre_process`,`post_process`}; scan these first when understanding macro usage

Applied to files:

  • tests/0083C150/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0F6DB678/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/**/*.fpp : Start module files with Fypp include for macros: `#:include 'macros.fpp'`

Applied to files:

  • tests/0083C150/golden-metadata.txt
  • tests/092A339C/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/04E1CB26/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/03D598D0/golden-metadata.txt
  • tests/11DAFD23/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/105673CA/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0ACB1F16/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/**/*.fpp : Use fypp macro `@:ALLOCATE(var1, var2)` for device-aware allocation instead of standard Fortran `allocate`

Applied to files:

  • tests/0083C150/golden-metadata.txt
  • tests/09623DE3/golden-metadata.txt
  • tests/07C33719/golden-metadata.txt
  • tests/02D1E522/golden-metadata.txt
  • tests/0853FBD1/golden-metadata.txt
  • tests/0A362971/golden-metadata.txt
  • tests/02B08B80/golden-metadata.txt
  • tests/0B07F3A1/golden-metadata.txt
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Mark GPU-callable helpers with `$:GPU_ROUTINE(function_name='...', parallelism='[seq]')` immediately after declaration

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Allocate large arrays with `managed` or move them into a persistent `$:GPU_ENTER_DATA(...)` region at start-up

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Use `private` declaration followed by explicit `public` exports in modules

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to **/*.{fpp,f90} : Limit routine arguments to ≤ 6; use derived-type params struct if more are needed

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Limit subroutines to ≤ 6 arguments; otherwise pass a derived-type 'params' struct

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:16.713Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T21:50:16.713Z
Learning: Applies to **/*.{fpp,f90} : Every argument must have explicit intent; use dimension/allocatable/pointer as appropriate

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Every variable must have `intent(in|out|inout)` specification and appropriate `dimension` / `allocatable` / `pointer`

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to **/*.{fpp,f90} : Use `wp` (working precision) parameter from `m_precision_select` instead of hardcoded precision like `real*8`

Applied to files:

  • src/simulation/m_data_output.fpp
📚 Learning: 2025-11-24T21:50:46.909Z
Learnt from: CR
Repo: MFlowCode/MFC PR: 0
File: .cursor/rules/mfc-agent-rules.mdc:0-0
Timestamp: 2025-11-24T21:50:46.909Z
Learning: Applies to src/simulation/**/*.{fpp,f90} : Declare loop-local variables with `private='[...]'` in GPU parallel loop macros

Applied to files:

  • src/simulation/m_data_output.fpp
🧬 Code graph analysis (1)
compare_goldenfiles.py (1)
toolchain/mfc/test/case.py (1)
  • TestCase (115-276)
🪛 GitHub Actions: Pretty
src/simulation/m_data_output.fpp

[error] 218-218: MFC formatting changes detected in m_data_output.fpp. Apply formatting and commit the changes.


[error] 298-298: MFC formatting changes detected in m_data_output.fpp. Apply formatting and commit the changes.

🪛 Ruff (0.14.6)
compare_goldenfiles.py

36-37: try-except-pass detected, consider logging the exception

(S110)


36-36: Do not catch blind exception: Exception

(BLE001)


46-46: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.

(S310)


52-52: Do not catch blind exception: Exception

(BLE001)


108-108: Do not catch blind exception: Exception

(BLE001)


161-161: Do not use bare except

(E722)


161-162: try-except-pass detected, consider logging the exception

(S110)


174-174: Do not use bare except

(E722)


174-175: try-except-pass detected, consider logging the exception

(S110)


200-200: f-string without any placeholders

Remove extraneous f prefix

(F541)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: Code Cleanliness Check
  • GitHub Check: Oak Ridge | Frontier (CCE) (gpu)
  • GitHub Check: Georgia Tech | Phoenix (NVHPC) (gpu)
  • GitHub Check: Georgia Tech | Phoenix (NVHPC) (gpu)
  • GitHub Check: Github (macos, mpi, debug, false)
  • GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
  • GitHub Check: Github (macos, mpi, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, no-debug, false)
  • GitHub Check: Github (ubuntu, mpi, debug, false)
  • GitHub Check: Github (ubuntu, mpi, debug, true)
  • GitHub Check: Self Hosted (gpu, omp, gt)
  • GitHub Check: Github (ubuntu, mpi, no-debug, true)
  • GitHub Check: Self Hosted (cpu, none, gt)
  • GitHub Check: Self Hosted (gpu, omp, frontier)
  • GitHub Check: Self Hosted (gpu, acc, gt)
  • GitHub Check: Self Hosted (gpu, acc, frontier)
  • GitHub Check: Self Hosted (cpu, none, frontier)
  • GitHub Check: Coverage Test on CodeCov
  • GitHub Check: Build & Publish
🔇 Additional comments (30)
tests/023DC5B9/golden-metadata.txt (1)

1-120: Clarify the rationale for the comprehensive environment metadata refresh.

The golden metadata has been updated with an entirely new build environment (different machine, compiler toolchain, CPU info). While this file should capture the build environment at test generation time, the PR objectives focus on probe testing logic changes—not environment/machine changes.

Per the learning, golden test outputs should be updated with clear justification. Please confirm:

  1. Was this metadata file intentionally regenerated on a new machine (Apple M1 Max with GNU gfortran vs. prior setup)?
  2. Were the actual probe test numerical results (not shown in this file) regenerated with the new code changes to verify correctness?
  3. Does the environment change impact the validity or reproducibility of the probe tests?

If this metadata refresh is incidental and the actual code changes are what drive the test results, consider clarifying the justification in the commit message.

tests/0A362971/golden-metadata.txt (1)

1-119: Clarify why golden metadata is being regenerated with machine-specific paths.

The entire golden-metadata.txt file has been regenerated with machine-specific paths (e.g., /Users/spencer/Downloads/MFC/... and /opt/homebrew/bin/gfortran), CMake version bumps, compiler toolchain changes, and CPU info from an Apple M1 Max device. This appears orthogonal to the PR objectives, which focus on probe functionality (bubble-probe readiness, ICFL checks, probe output format fixes).

Golden metadata files containing absolute user paths and machine-specific details may cause issues:

  • Other developers running tests locally will generate different metadata
  • CI/CD systems will have different paths and toolchains
  • Golden file diffs become polluted with environment noise rather than functional changes

Questions:

  1. Is this metadata regeneration intentional, or was it accidentally committed?
  2. If intentional, what is the justification for checking in user-specific paths?
  3. Should golden metadata be normalized (e.g., using <USER>, <HOME>, or relative paths) or excluded from version control?

Based on learnings, golden test outputs should only be updated with clear justification. If the build environment changes are essential to validating the probe changes, this should be clearly documented in the PR description.

tests/0288BDAD/golden-metadata.txt (1)

1-120: Verify justification for golden-metadata regeneration.

The entire golden-metadata.txt file has been regenerated with environment-specific details (Apple M1 Pro host, AppleClang/GNU compilers, macOS Fypp path, Apple M1 Max CPU info). However, the PR objectives focus on bubble probe readiness, ICFL checks, and probe output fixes—none directly related to system metadata.

This regeneration appears to reflect local developer environment differences rather than changes required by the PR's functional objectives. Per the coding guidelines, golden test outputs should not be updated without clear justification and should remain stable to prevent CI environment mismatches.

Please clarify:

  1. Is this metadata regeneration intentional, or did it occur as a side effect of running tests locally?
  2. Does the target CI environment match this new configuration (host, compilers, paths)?
  3. Are there actual functional test result changes (not just metadata) driven by the probe-related code changes that would warrant updating golden outputs?

If the regeneration is necessary, please update the PR description to document why and ensure the CI environment is compatible.

tests/03D598D0/golden-metadata.txt (1)

1-119: Verify environmental and toolchain changes align with PR objectives.

This golden metadata file shows extensive environment/toolchain/hardware updates (Apple Silicon M1 Max, CMake v4.1.2, new compiler paths, OpenMP OFF), but the PR objectives focus exclusively on bubble probe initialization and ICFL checks. The changes appear disconnected from the stated PR goals.

Key concerns:

  • Why did the test environment change from the prior hardware/toolchain baseline?
  • Is the switch to Apple M1 Max intentional and necessary for the probe fixes?
  • Are the OpenMP OFF changes related to probe functionality or unrelated?
  • Should these environment updates be part of this PR or handled separately?

The "dirty" git state (Line 7) also warrants clarification.

Please confirm that:

  1. These environmental/toolchain changes are intentional and necessary for the bubble probe fixes.
  2. The new hardware (Apple M1 Max) is the correct baseline for this repository going forward.
  3. The OpenMP OFF setting is expected and related to the probe fix scope.
  4. The git "dirty" state is expected or should be resolved.

If these changes are environment rebaselining unrelated to the probe fixes, consider splitting them into a separate PR for clarity.

tests/07C54EDD/golden-metadata.txt (1)

1-119: Clarify whether golden file regeneration in --generate mode is expected behavior for this test.

This golden metadata file contains system-specific data (CMake version, compiler paths, CPU info). Before flagging this as unjustified, confirm whether:

  • The test -j 10 --generate invocation is intended to regenerate golden files as part of normal test operations
  • Golden file regeneration is standard for this test case
  • The PR changes justify updating the metadata baseline

If golden file regeneration is expected for --generate mode, the original concern may not apply.

tests/0387594F/golden-metadata.txt (1)

1-120: Golden test file regeneration requires clear justification and verification of numeric results.

This golden metadata file was regenerated in a different environment (M1 Pro → M1 Max, compiler versions, CMake v4.1.2) per the PR's batch golden file refresh. Per project guidelines, ensure:

  1. Numeric golden output files for test 0387594F changes are intentional consequences of bubble probe fixes, not unrelated drift
  2. Golden file regeneration aligns with stated PR objectives and introduces no unexpected regressions
tests/07C33719/golden-metadata.txt (2)

1-1: Justify metadata re-baselining against PR objectives.

The golden-metadata file has been updated with new timestamps, test invocation parameters (-j 10 --generate), toolchain versions (CMake v4.1.2, AppleClang v17.0.0.17000404, GNU v15.2.0), and CPU metadata (Apple M1 Max). While these changes reflect an environment update, the PR objectives focus on functional changes: bubble probe readiness gating, ICFL check relaxation, probe format fixes, and probe write gating.

Per the learnings, ensure no regressions in golden test outputs without clear justification. Confirm that this metadata update is intentional and aligned with test infrastructure changes (not incidental re-baselining on a different developer machine). If the functional changes in the PR require updated test reference data, provide that context.

Also applies to: 5-7


27-27: Verify that OpenMP OFF is intentional.

OpenMP is consistently set to OFF across syscheck, pre_process, and simulation blocks. Confirm whether this is a deliberate configuration change for this test case or an incidental side-effect of the environment update.

Also applies to: 61-61, 95-95

tests/0FCCE9F1/golden-metadata.txt (1)

1-119: Clarify whether machine-specific metadata should be version-controlled.

This golden metadata file captures environment-specific details including absolute user paths (/Users/spencer/Downloads/MFC/...), machine name (shb-m1pro-3.local), and hardware configuration (Apple M1 Max). If this file is committed to version control, it will lock the test validation to a specific developer machine, causing failures when run on CI/CD systems or other machines with different paths and configurations.

Verify the following:

  • Is this file meant to be golden/canonical test output, or a local build artifact?
  • If version-controlled, how does the test framework handle environment-specific differences across CI/CD and developer machines?
  • Does the PR description justify why environment metadata changes are necessary for the stated bubble probe and ICFL objectives?
tests/09623DE3/golden-metadata.txt (3)

5-6: Justify test invocation and lock flag changes.

The test invocation changed to test -j 10 --generate with lock flags. The PR description does not mention why parallelization or regeneration flags were applied. Clarify whether:

  • Parallel test execution (-j 10) is intentional for this case, or a side effect of local testing.
  • Lock flags (mixed=No & fastmath=No) reflect required build configuration for the probe changes.

27-27: OpenMP set to OFF across all build targets; justification needed.

OpenMP is toggled OFF in syscheck, pre_process, and simulation targets. The PR objectives do not mention OpenMP changes. Verify:

  • Is this an intentional change required by the bubble probe modifications?
  • Or is this an environmental artifact of the Apple M1 build configuration?

Also applies to: 61-61, 95-95


1-7: Golden file update requires justification; verify numeric outputs remain correct and clarify environmental scope.

This golden metadata file reflects a significant environment change (Apple M1 Mac, different compiler versions, new host path), but the PR focuses on bubble probe readiness and output format fixes—not environmental updates. Per established learnings, golden test outputs must be regenerated with clear justification and verified for regressions.

Please clarify:

  • Was regenerating this golden file necessary to support the probe-testing changes, or does it reflect only environmental drift?
  • Did the code changes cause numeric test output differences, or is this purely an environmental shift?
  • Should absolute developer paths (e.g., /Users/spencer/Downloads/MFC/...) be committed to golden metadata, or should these be normalized?
tests/0D1FA5C5/golden-metadata.txt (1)

1-1: Verify necessity of golden metadata regeneration.

This golden metadata file has been regenerated with environment-specific details (timestamp, Apple Silicon CPU, user-specific paths like /Users/spencer/Downloads/MFC/build/venv/bin/fypp, macOS toolchain versions). Based on learnings, golden test outputs should only change when functionally required.

The PR objectives focus on bubble probe readiness and ICFL calculation improvements. Confirm whether these functional changes necessitate regenerating test baselines, or if these metadata updates are incidental artifacts from local testing on a different machine.

Per project learnings: "ensure no regressions in golden test outputs without clear justification"

Also applies to: 5-7, 13-16, 29-29, 36-38, 63-63, 97-97, 114-119

tests/11DAFD23/golden.txt (1)

4-4: LGTM! Golden data updated for probe output changes.

The numerical updates in cons/prim data files and the new probe1_prim.dat data block appropriately reflect the probe-related functional changes described in the PR objectives (probe output format fixes, bubble probe readiness).

Based on PR objectives describing probe output format string fixes and bubble probe initialization changes.

Also applies to: 6-6, 16-16, 18-18, 25-25

src/common/m_variables_conversion.fpp (1)

1641-1654: Division-by-zero guards added for bubble simulations with void fraction clamping.

The changes guard against division by zero when void fraction approaches 1.0 by clamping denominators with small_alf. The added denom variable prevents overflow when (1 - alpha_gas) is extremely small. Code follows required style conventions: 2-space indentation, lowercase keywords, real(wp) precision specification, and clear explanatory comments.

tests/07D9EBD1/golden-metadata.txt (1)

1-120: Verify that golden test numerical outputs justify the metadata rebase to Apple Silicon.

All 8 golden-metadata.txt files have been synchronized to a new Apple Silicon/macOS environment (shb-m1pro-3.local, Apple M1 Max, AppleClang + GNU Fortran). The metadata reflects an environment migration rather than code-driven changes.

Per learnings, golden test outputs should not regress without clear justification. The PR's functional changes (bubble_probe_ready flag, ICFL relaxation, probe format fixes, probe writing delays) should directly justify any changes to numerical golden outputs. However, this review covers metadata files only—the actual numerical golden outputs require inspection to confirm they changed only where PR objectives justify.

Confirm that:

  1. The actual golden numerical outputs (not just metadata) have changed only where PR objectives justify the changes.
  2. All tests pass with the new environment and functional changes.
  3. There are no unexpected regressions in simulation results.
tests/0879E062/golden-metadata.txt (1)

1-120: Environment-only metadata updates with no functional changes; request justification.

These 8 golden-metadata files all reflect an environment rebaseline to Apple Silicon/macOS (CMake v4.1.2, AppleClang/GNU Fortran toolchains, Apple M1 Max CPU descriptors, Fypp paths). The metadata is accurate for the new host environment but appears decoupled from the probe-testing feature described in the PR objectives (bubble-probe readiness, ICFL relaxation, probe output format fixes).

Per the learnings provided, golden file updates should have clear justification. While environment-driven changes are expected when tests migrate hosts, wholesale metadata updates across many test files merit clarification:

  1. Scope: Are these environmental updates foundational to the probe-testing changes, or are they orthogonal?
  2. Justification: Do the probe-testing functional changes require these metadata updates, or are they convenience updates?
  3. Coverage: Numeric golden files (cons/prim/probe data) should show the actual probe-testing behavior changes; the metadata alone does not demonstrate correctness of the feature.
src/simulation/m_data_output.fpp (9)

1893-1899: Bubble probe readiness gating logic is correctly implemented.

The initialization properly distinguishes between bubble and non-bubble cases, ensuring probes are ready immediately for non-bubble simulations while deferring for bubble cases until explicit readiness marking.


229-238: Probe file initialization correctly gated for bubble simulations.

The early return for unready bubble probes and explicit D directory creation ensure proper initialization sequencing.


300-304: Runtime information writing correctly gated for bubble cases.

The early return prevents stability criteria calculation before bubble initialization, consistent with the probe readiness pattern.


390-400: ICFL sanity checks are correctly enforced after bubble initialization.

Contrary to a past review concern, these checks DO execute for bubble simulations once bubble_probe_ready is true (the function returns early at lines 302-304 otherwise). The logic correctly ensures checks run post-initialization.

Note: This addresses the past review comment about ICFL safety checks being bypassed.


1123-1123: Probe file signature correctly updated to remove accel_mag parameter.

The signature change aligns with the PR objective to streamline probe output for bubble simulations.


1236-1239: Probe indexing conversion is correctly implemented across all dimensions.

The conversion from 1-based minloc results to 0-based q_cons_vf indices is properly handled:

  • Accounts for distx/y/z arrays starting at -1
  • Clamps to valid q_cons_vf range [0:m/n/p]
  • Subsequent usages consistently apply indices directly without additional offsets

This addresses the past review concern about "two cells left" offset, which applied to older code.

Also applies to: 1375-1380, 1483-1490


1307-1354: Bubble statistics computation properly guarded against numerical instability.

The defensive checks prevent division by zero when volume fraction or bubble number density is negligible, with appropriate fallback to zero. This ensures robust handling of edge cases during bubble initialization and low-population regions.


1876-1878: Probe file closing correctly gated for bubble cases.

The early return prevents attempting to close probe files that were never opened due to bubble initialization delays.


218-218: Manual verification required: unable to access repository to confirm formatting issues.

The CI pipeline detected formatting inconsistencies at lines 218 and 296-298, but these cannot be verified without accessing the file. Run the MFC formatter locally and inspect the changes to confirm they match the coding guidelines for .fpp files (2-space indentation, lowercase keywords, proper alignment of continuation lines under &). If formatting passes locally, commit the changes.

tests/10DBA76B/golden-metadata.txt (1)

1-154: Golden test baselines should be generated in a consistent CI environment, not on local developer machines.

The updated golden metadata includes developer-specific paths (/Users/spencer/Downloads/MFC/...), local machine names (shb-m1pro-3.local, MacBookPro), and a dirty Git state. Golden files serve as regression test baselines and should be generated in a reproducible, standardized environment rather than on individual developer machines to ensure consistency across test runs and CI pipelines.

tests/0A62F0A6/golden-metadata.txt (1)

1-7: Verify intent of test baseline shift to Apple Silicon environment.

The metadata reflects a complete environment transition from x86_64/Intel toolchains to Apple Silicon/macOS (M1 Max) with updated CMake (v4.1.2), compilers (AppleClang, GNU Fortran v15.2.0), and Fypp paths. This baseline shift isn't mentioned in the PR objectives, which focus on bubble-probe readiness logic.

Per learning guidelines, golden test outputs should only change with clear justification. Clarify:

  1. Was this environment shift intentional for this PR?
  2. Do the numeric golden test files (golden.txt) also reflect regenerated data?
  3. Are there numeric regressions or is this purely a metadata/environment refresh?
tests/02748F0F/golden-metadata.txt (1)

1-7: Consistent environment baseline shift across multiple tests — requires justification.

This metadata file shows the same Apple Silicon environment baseline shift observed in tests/0A62F0A6/golden-metadata.txt. The identical toolchain details (AppleClang, GNU Fortran v15.2.0, CMake v4.1.2) and CPU descriptors suggest coordinated test regeneration on a single macOS machine.

Based on learnings provided, ensure no regressions in golden test outputs without clear justification. Document why the entire test suite baseline was regenerated on a different platform as part of this PR.

tests/04E1CB26/golden-metadata.txt (1)

1-7: Batch test regeneration on macOS — clarify scope and justification.

The repeated pattern across multiple metadata files with consistent timestamps in a narrow window (22:48-22:55 UTC) suggests a coordinated batch regeneration of the test suite on the macOS Apple Silicon machine.

Verify: Were all tests regenerated as part of this PR, or only bubble-related test cases? If all tests were regenerated, clarify the PR's scope to include this environment transition.

OpenMP : OFF

Fypp : /Users/spencer/Downloads/misc-MFC/MFC/build/venv/bin/fypp
Fypp : /Users/spencer/Downloads/MFC/build/venv/bin/fypp
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Non-portable user-specific path in Fypp configuration.

Lines 29, 63, and 97 contain an absolute path specific to a developer machine: /Users/spencer/Downloads/MFC/build/venv/bin/fypp. This will not resolve on CI systems, other developers' machines, or production environments.

Consider replacing this with a relative path, environment variable reference, or system-level Fypp installation path to ensure portability.

Also applies to: 63-63, 97-97

🤖 Prompt for AI Agents
In tests/07C33719/golden-metadata.txt lines 29, 63, and 97 there is a
hard-coded, user-specific absolute path
(/Users/spencer/Downloads/MFC/build/venv/bin/fypp) which is non-portable;
replace these entries with a portable reference such as a relative path inside
the repo, a PATH-resolvable command name ("fypp"), or an
environment-variable-based path (e.g., ${VENV}/bin/fypp or ${FYPP_BIN}) and
update the test/golden file accordingly so CI and other developer machines can
resolve Fypp without relying on a single developer's home directory.

OpenMP : OFF

Fypp : /Users/spencer/Downloads/misc-MFC/MFC/build/venv/bin/fypp
Fypp : /Users/spencer/Downloads/MFC/build/venv/bin/fypp
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace absolute developer paths with portable environment references.

Fypp path is set to an absolute developer-specific path (/Users/spencer/Downloads/MFC/build/venv/bin/fypp). Golden files should be portable across machines and CI environments.

Consider using environment-relative or build-relative paths (e.g., ${BUILD_DIR}/venv/bin/fypp or similar) to ensure reproducibility and portability.

Also applies to: 63-63, 97-97

🤖 Prompt for AI Agents
In tests/09623DE3/golden-metadata.txt around line 29 (and also lines 63 and 97),
the Fypp entry uses an absolute developer path
(/Users/spencer/Downloads/MFC/build/venv/bin/fypp); update these entries to use
a portable environment or build-relative reference (for example
${BUILD_DIR}/venv/bin/fypp or ${VENV_BIN}/fypp) so the golden file is machine-
and CI-independent, and ensure any CI harness substitutes the chosen variable at
test runtime.

@sbryngelson sbryngelson marked this pull request as draft December 1, 2025 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant