Remove dead ACC loop directives in s_derive_center_of_mass

[sbryngelson]

Summary

• In s_derive_center_of_mass (m_derived_variables.fpp), two $:GPU_LOOP(parallelism='[seq]') directives appeared before the c_m zero-initialization loops, outside any GPU parallel region.
• These expanded to !$acc loop seq with no surrounding !$acc parallel/!$acc kernels, so the compiler silently ignored them. CCE 19.0.0 correctly warns: ftn-7255: loop marked for accelerator worksharing but not inside an accelerator region.
• The initialization runs on CPU and is pushed to device via the existing $:GPU_UPDATE(device='[c_m]') immediately after — the structure is correct, the directives were just dead code.
• Replace the 7-line nested loop with c_m(:, :) = 0.0_wp, which is idiomatic Fortran and eliminates the spurious directives.

Test plan

• Verify ftn-7255 warning is gone from the CCE 19.0.0 (cpe/25.03) build of simulation
• Confirm s_derive_center_of_mass produces identical results (center-of-mass values unchanged)

🤖 Generated with Claude Code

[github-actions]

Claude Code Review

Head SHA: b1dc5c9

Files changed: 1

• src/simulation/m_derived_variables.fpp

Summary:

• Removes 7 lines of dead GPU loop directives and replaces them with 1 idiomatic Fortran array assignment
• The original code used $:GPU_LOOP(parallelism='[seq]') directives outside any GPU parallel region — they expanded to dangling ! loop seq without a surrounding ! parallel/! kernels, making them no-ops that triggered a CCE 19.0.0 compiler warning (ftn-7255)
• The replacement c_m(:, :) = 0.0_wp is correct Fortran, removes the spurious directives, and leaves the existing $:GPU_UPDATE(device='[c_m]') downstream untouched — the semantics are unchanged
• Net change: -6 lines, cleaner code, silences a legitimate compiler warning

Findings:

No issues found. The change is correct and minimal.

Improvement opportunities (optional):

• None — this is as clean as it gets for this fix.

[qodo-code-review]

Review Summary by Qodo

Remove dead ACC loop directives in s_derive_center_of_mass

🐞 Bug fix

Walkthroughs

Description

• Remove dead ACC loop directives causing compiler warnings
• Replace nested loop with idiomatic Fortran array assignment
• Simplify c_m initialization while maintaining correctness

Diagram

flowchart LR
  A["Nested loop with GPU_LOOP directives"] -->|"Replace with"| B["Array assignment c_m = 0.0"]
  B -->|"Result"| C["Eliminates ftn-7255 warning"]
  C -->|"Maintains"| D["Correct GPU_UPDATE behavior"]

Loading

File Changes

1. src/simulation/m_derived_variables.fpp 🐞 Bug fix +1/-7

Simplify c_m initialization and remove dead directives

• Removed two dead $:GPU_LOOP(parallelism='[seq]') directives from c_m initialization
• Replaced 7-line nested loop with single-line array assignment c_m(:, :) = 0.0_wp
• Eliminates CCE 19.0.0 compiler warning ftn-7255 about loop directives outside GPU region
• Preserves existing $:GPU_UPDATE(device='[c_m]') that handles device synchronization

src/simulation/m_derived_variables.fpp

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

ⓘ The new review experience is currently in Beta. Learn more

[github-actions]

Claude Code Review

Head SHA: e96a3b8

Files changed: 1

• src/simulation/m_derived_variables.fpp (+1 / -7)

Summary

• Removes two spurious $:GPU_LOOP(parallelism='[seq]') directives used outside any GPU parallel region, which CCE 19.0.0 correctly flags as ftn-7255.
• Replaces a 7-line nested explicit loop initializing c_m to zero with the idiomatic Fortran whole-array assignment c_m(:, :) = 0.0_wp.
• The $:GPU_UPDATE(device='[c_m]') that follows is retained and remains correct — it pushes the CPU-initialized array to the device before the GPU parallel region.
• Net result: cleaner code, warning eliminated, no behavioral change.

Findings

No correctness issues. The change is correct and minimal:

1. Dead directives correctly identified (m_derived_variables.fpp, ~line 428): $:GPU_LOOP outside a GPU_PARALLEL or GPU_PARALLEL_LOOP region is indeed dead/invalid code. The compiler warning is accurate.

2. Whole-array assignment is safe here: c_m is a 2D array bounded by num_fluids × 5. The whole-array form c_m(:, :) = 0.0_wp is equivalent, more readable, and lets the compiler optimize freely.

3. Precision compliance: 0.0_wp is correct per project conventions (no d exponent literals, no bare 0.0).

4. GPU_UPDATE placement is unchanged and still correct: host initializes → GPU_UPDATE(device=...) → GPU parallel region reads. No MPI or memory-management issues introduced.

Opportunities

• None beyond what this PR already addresses. The fix is minimal and appropriate.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 22fb3b0e-7864-4902-bb3a-31f7242a6f11

📥 Commits

Reviewing files that changed from the base of the PR and between 506c6f5 and e96a3b8.

📒 Files selected for processing (1)

• src/simulation/m_derived_variables.fpp

---

📝 Walkthrough

Walkthrough

The file src/simulation/m_derived_variables.fpp has been modified to change the initialization approach for the CoM buffer. Previously, nested loops iterated over fluids and components to set each c_m(i, j) element to 0.0_wp. This multi-loop structure has been replaced with a single bulk assignment statement c_m(:, :) = 0.0_wp, which directly initializes all array elements. The subsequent GPU\_UPDATE call is unchanged. This modification reduces the code from 7 lines to 1 line for this initialization operation.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: removing dead ACC loop directives from the s_derive_center_of_mass subroutine, which matches the core objective of the PR.
Description check	✅ Passed	The description provides comprehensive context about the problem (dead directives causing compiler warnings), the solution (replace nested loop with array assignment), and a test plan, though the description template sections are not formally structured.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Claude Code Review

Head SHA: e96a3b8

Files changed: 1

• src/simulation/m_derived_variables.fpp

Summary:

• Removes two $:GPU_LOOP(parallelism='[seq]') directives that appeared outside any GPU parallel region in s_derive_center_of_mass
• These directives expanded to !\ loop seq with no enclosing !\ parallel/!\ kernels, making them dead code (CCE 19.0.0 correctly flags this as warning ftn-7255)
• Replaces the 7-line nested initialization loop with idiomatic c_m(:, :) = 0.0_wp, which is cleaner and consistent with running on the CPU before the subsequent $:GPU_UPDATE(device='[c_m]')
• No functional change — behavior is identical; only the spurious directives are removed

Findings:

No correctness issues found.

• src/simulation/m_derived_variables.fpp (removed ~L428–434): The original $:GPU_LOOP calls outside a GPU_PARALLEL_LOOP / GPU_PARALLEL region are correctly identified as misplaced per the GPU macro rules (and the rule that GPU_LOOP emits empty directives on Cray/AMD, causing silent serial execution anyway). Removing them is the right call.
• c_m(:, :) = 0.0_wp (new L428): Precision is correct (wp kind), array syntax is idiomatic. The existing $:GPU_UPDATE(device='[c_m]') immediately after preserves device coherence. No issues.

Improvement opportunity (minor, optional):
c_m = 0.0_wp (without explicit (:, :)) is equally valid and slightly more concise, but the explicit subscript form is not wrong — just a style preference.

Overall: clean, minimal, and correct fix. Approved. ✓

[Copilot]

Pull request overview

This PR removes dead/ineffective GPU loop directives in s_derive_center_of_mass and replaces a nested zero-initialization loop with an idiomatic Fortran whole-array assignment, eliminating a CCE warning about accelerator worksharing outside an accelerator region.

Changes:

• Replace CPU-side nested loops initializing c_m with c_m(:, :) = 0.0_wp.
• Remove two $:GPU_LOOP(parallelism='[seq]') directives that expanded to ignored !$acc loop seq outside an accelerator region.

[qodo-code-review]

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Remove dead ACC loop directives in s_derive_center_of_mass

🐞 Bug fix

Walkthroughs

Description

• Remove dead ACC loop directives causing CCE compiler warnings
• Replace nested initialization loop with idiomatic array assignment
• Simplify code while maintaining identical functionality

Diagram

flowchart LR
  A["Nested loop with<br/>GPU_LOOP directives"] -->|"Replace with"| B["Array assignment<br/>c_m = 0.0"]
  B -->|"Result"| C["No compiler warnings<br/>Same functionality"]

Loading

File Changes

1. src/simulation/m_derived_variables.fpp 🐞 Bug fix +1/-7

Simplify c_m initialization and remove dead directives

• Removed two dead $:GPU_LOOP(parallelism='[seq]') directives from s_derive_center_of_mass
 subroutine
• Replaced 7-line nested loop structure with single array assignment c_m(:, :) = 0.0_wp
• Eliminates CCE 19.0.0 ftn-7255 compiler warning about loop directives outside GPU parallel region
• Maintains identical functionality and GPU update behavior via existing
 $:GPU_UPDATE(device='[c_m]')

src/simulation/m_derived_variables.fpp

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

ⓘ The new review experience is currently in Beta. Learn more

[github-actions]

Claude Code Review

Head SHA: e96a3b8

Files changed: 1

• src/simulation/m_derived_variables.fpp

Summary:

• Removes 7 lines of dead code (two spurious $:GPU_LOOP(parallelism='[seq]') directives wrapping a c_m zero-initialization loop outside any GPU parallel region)
• Replaces the nested do loops with a single idiomatic array assignment c_m(:, :) = 0.0_wp
• Eliminates CCE 19.0.0 warning ftn-7255: loop marked for accelerator worksharing but not inside an accelerator region
• The subsequent $:GPU_UPDATE(device='[c_m]') correctly syncs the host-initialized array to device, so semantics are unchanged

Findings:

No correctness issues found. The change is straightforward and safe:

1. Correctness (m_derived_variables.fpp:434): The $:GPU_LOOP macros here expand to ! loop seq (or equivalent) with no enclosing ! parallel/! kernels region, so they were already dead/ignored. The CPU-side initialization path is preserved, and the existing $:GPU_UPDATE(device='[c_m]') pushes the zeroed values to the device correctly. Behavior is identical.

2. Style: c_m(:, :) = 0.0_wp is standard idiomatic Fortran for whole-array initialization and is cleaner than the explicit nested loops. No concerns.

3. Precision: Uses 0.0_wp correctly (not 0.0d0 or a bare 0.0). Compliant with the precision discipline rules.

No issues found. This is a clean removal of dead/incorrect GPU directives with no functional change. Ready to merge.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.95%. Comparing base (506c6f5) to head (e96a3b8).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1282   +/-   ##
=======================================
  Coverage   44.95%   44.95%           
=======================================
  Files          70       70           
  Lines       20504    20503    -1     
  Branches     1946     1946           
=======================================
  Hits         9217     9217           
+ Misses      10166    10165    -1     
  Partials     1121     1121           

☔ 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.