Make fluid variables uniform through the code

[JRChreim]

User description

User description

Description

The following PR comprises changes to the code to call fluid variables uniformly throughout the code. The main changes for this PR is to change all the fluid_pp(i)% to their respective variables, defined on src/common/m_variables_conversion.fpp. For instance, substituting fluid_pp(i)%pi_inf by pi_infs(i), and so on.

Type of change

• Something else

Scope

• This PR comprises a set of related changes with a common goal

If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.

• I ran ./mfc.sh format before committing my code
• New and existing tests pass locally with my changes, including with GPU capability enabled (both NVIDIA hardware with NVHPC compilers and AMD hardware with CRAY compilers) and disabled
• This PR does not introduce any repeated code (it follows the DRY principle)
• I cannot think of a way to condense this code and reduce any introduced additional line count

If your code changes any code source files (anything in src/simulation)

To make sure the code is performing as expected on GPU devices, I have:

• Checked that the code compiles using NVHPC compilers
• Checked that the code compiles using CRAY compilers
• Ran the code on either V100, A100, or H100 GPUs and ensured the new feature performed as expected (the GPU results match the CPU results)
• Ran the code on MI200+ GPUs and ensure the new features performed as expected (the GPU results match the CPU results)
• Enclosed the new feature via nvtx ranges so that they can be identified in profiles
• Ran a Nsight Systems profile using ./mfc.sh run XXXX --gpu -t simulation --nsys, and have attached the output file (.nsys-rep) and plain text results to this PR
• Ran a Rocprof Systems profile using ./mfc.sh run XXXX --gpu -t simulation --rsys --hip-trace, and have attached the output file and plain text results to this PR.
• Ran my code using various numbers of different GPUs (1, 2, and 8, for example) in parallel and made sure that the results scale similarly to what happens if you run without the new code/feature

---

PR Type

Enhancement

---

Description

• Replace fluid_pp(i)% struct accesses with uniform variable arrays

• Substitute gamma, pi_inf, qv with gammas(i), pi_infs(i), qvs(i)

• Replace index calculations with shorthand variables (intxb, advxb, contxb, gs_min, ps_inf)

• Remove redundant local variable allocations from pressure relaxation module

• Add missing module imports for variable conversion support

---

Diagram Walkthrough

flowchart LR
  A["fluid_pp struct<br/>component access"] -->|"Replace with"| B["Uniform variable<br/>arrays"]
  B --> C["gammas, pi_infs,<br/>qvs arrays"]
  D["Index calculations<br/>e.g. i+adv_idx%beg-1"] -->|"Replace with"| E["Shorthand variables<br/>intxb, advxb, contxb"]
  F["Local gamma_min,<br/>pres_inf arrays"] -->|"Remove"| G["Use gs_min, ps_inf<br/>from m_variables_conversion"]

Loading

File Walkthrough

	Relevant files
Enhancement	9 files m_variables_conversion.fpp Replace fluid_pp struct with uniform variable arrays          +5/-8      m_data_output.fpp Update fluid variable access to use uniform arrays              +6/-4      m_derived_variables.fpp Standardize fluid property variable references                      +4/-4      m_assign_variables.fpp Replace fluid_pp struct with uniform variable arrays          +5/-5      m_data_output.fpp Update fluid variable access patterns uniformly                    +4/-4      m_icpp_patches.fpp Add module import and standardize variable access                +15/-13  m_data_output.fpp Replace fluid_pp struct with uniform variable arrays          +6/-6      m_pressure_relaxation.fpp Remove redundant local arrays use global variables              +7/-19    m_start_up.fpp Standardize internal energy calculation variable access    +2/-3
Miscellaneous	1 files m_global_parameters.fpp Remove debug print statements                                                        +0/-2

---

---

CodeAnt-AI Description

Ensure derived outputs use shared EOS parameters

What Changed

• Mixes, energy diagnostics, and output routines now read gammas, pi_inf, and qv from the shared EOS arrays, making energies, sound speeds, and post-processed reports follow any updated EOS configuration.
• Initial condition patches and internal energy initialization use the shared EOS values so generated densities and energies match the global conversion tables instead of stale fluid records.
• Pressure-relaxation now enforces bounds and iterates with the shared EOS parameters, keeping pressures and derived volume fractions within the corrected range even when EOS values shift.

Impact

✅ Consistent EOS-driven energy and sound-speed reports
✅ Valid pressure and volume-fraction recovery during relaxation
✅ Accurate EOS-based initial conditions and diagnostics

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

• Refactor

  • Centralized EOS and thermodynamic parameter access across simulation, pre/post‑processing, probes, and output to ensure consistent energy, sound‑speed, initialization, and relaxation computations.
  • Updated derived-variable, internal‑energy and probe/output calculations to read from the consolidated parameter set for uniform behavior.

• Chores

  • Removed obsolete debug output and cleaned up redundant/deprecated parameter declarations.

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

[codeant-ai]

CodeAnt AI is reviewing your PR.

[coderabbitai]

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

This PR centralizes EOS parameter usage by replacing per-fluid fluid_pp(...) property references with shared arrays (gammas, pi_infs, qvs) and helper arrays (gs_min, ps_inf) across pre-process, simulation, post-process, and common modules; it also removes now-redundant module-level arrays and a couple commented debug prints.

Changes

Cohort / File(s)	Summary
Core conversion updates src/common/m_variables_conversion.fpp	Consolidated energy reconstruction expression (qv appended consistently); switched conservative→primitive index bases to intxb/contxb; replaced fluid_pp(...) gamma/pi_inf/qv with gammas/pi_infs/qvs; use gs_min in sound-speed contribution.
Post-process updates src/post_process/m_data_output.fpp, src/post_process/m_derived_variables.fpp	Added use m_variables_conversion; replaced fluid_pp(...)%gamma/%pi_inf/%qv with gammas/pi_infs/qvs; used gs_min for lit-gamma / block-modulus contributions.
Pre-process updates & patches src/pre_process/m_assign_variables.fpp, src/pre_process/m_data_output.fpp, src/pre_process/m_icpp_patches.fpp	Replaced fluid_pp(...) EOS fields with gammas/pi_infs/qvs and gs_min; recomputed lit_gamma from gs_min; added use m_variables_conversion.
Simulation-side migrations src/simulation/m_data_output.fpp, src/simulation/m_start_up.fpp	Read EOS parameters from gammas(1), pi_infs(1), qvs(1), and gs_min(1) instead of fluid_pp(1); updated startup internal-energy loop indices to use intxb/advxb/contxb and new arrays.
Pressure-relaxation cleanup src/simulation/m_pressure_relaxation.fpp	Removed allocatable gamma_min and pres_inf declarations and GPU directives; replaced uses with gs_min and ps_inf across initialization and Newton–Raphson routines; propagated replacements through bounds and exponent expressions.
Minor cleanup src/simulation/m_global_parameters.fpp	Removed two commented debug print statements (no functional change).

Sequence Diagram(s)

(Skipped — changes are parameter-source substitutions and small refactors; control flow and component interactions remain unchanged.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Files needing careful verification:
  • src/common/m_variables_conversion.fpp — energy & sound-speed math correctness after variable/index substitutions.
  • src/simulation/m_start_up.fpp — index-base substitutions (intxb/advxb/contxb) and correct mapping to conserved/primitive offsets.
  • src/simulation/m_pressure_relaxation.fpp — removal of allocatables and correctness of replacements to gs_min/ps_inf.
  • Cross-module uses of gammas/pi_infs/qvs/gs_min/ps_inf — initialization and lifetime.

Possibly related PRs

• Complete form of the SG EoS #1057 — overlaps on m_variables_conversion and qv/gs_min/ps_inf propagation and related energy/sound-speed fixes.
• Add line numbering to gpu loops #1029 — touches the same conservative↔primitive conversion code paths and related index/loop logic in m_variables_conversion.

Suggested reviewers

• wilfonba

Poem

🐰 I hopped through variables, neat and spry,
Gammas, pi_infs, qvs stacked high.
gs_min glows, ps_inf tucked in view,
One fluffy refactor — tidy and true. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is comprehensive and well-structured, covering the main changes, scope, testing performed, and checklists. It includes a clear diagram and file walkthrough. However, GPU-specific compilation and profiling checks remain unchecked.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title clearly describes the main objective: making fluid variables uniform across the codebase by replacing struct-style accesses with consistent array variables.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

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

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue Pressure lower-bound clamp changed from using per-fluid pres_inf to ps_inf; verify ps_inf is correctly defined/initialized and has identical units/meaning, or this may alter stability and violate EOS constraints. ! Initialize pressures pres_relax = 0._wp $:GPU_LOOP(parallelism='[seq]') do i = 1, num_fluids if (q_cons_vf(i + advxb - 1)%sf(j, k, l) > sgm_eps) then pres_K_init(i) = (q_cons_vf(i + intxb - 1)%sf(j, k, l)/ & q_cons_vf(i + advxb - 1)%sf(j, k, l) - pi_infs(i))/gammas(i) if (pres_K_init(i) <= -(1._wp - 1.e-8_wp)*ps_inf(i) + 1.e-8_wp) & pres_K_init(i) = -(1._wp - 1.e-8_wp)*ps_inf(i) + 1.e-8_wp else pres_K_init(i) = 0._wp end if pres_relax = pres_relax + q_cons_vf(i + advxb - 1)%sf(j, k, l)*pres_K_init(i) end do ! Newton-Raphson iteration f_pres = 1.e-9_wp df_pres = 1.e9_wp $:GPU_LOOP(parallelism='[seq]') do iter = 0, MAX_ITER - 1 if (abs(f_pres) > TOLERANCE) then pres_relax = pres_relax - f_pres/df_pres ! Enforce pressure bounds do i = 1, num_fluids if (pres_relax <= -(1._wp - 1.e-8_wp)*ps_inf(i) + 1.e-8_wp) & pres_relax = -(1._wp - 1.e-8_wp)*ps_inf(i) + 1._wp end do Algorithm Change Newton update now uses gs_min in exponents/derivatives instead of gamma_min; confirm gs_min equals (1/gamma+1) and matches prior theory, else convergence and mixture density updates may be incorrect. ! Newton-Raphson step f_pres = -1._wp df_pres = 0._wp $:GPU_LOOP(parallelism='[seq]') do i = 1, num_fluids if (q_cons_vf(i + advxb - 1)%sf(j, k, l) > sgm_eps) then rho_K_s(i) = q_cons_vf(i + contxb - 1)%sf(j, k, l)/ & max(q_cons_vf(i + advxb - 1)%sf(j, k, l), sgm_eps) & *((pres_relax + ps_inf(i))/(pres_K_init(i) + & ps_inf(i)))**(1._wp/gs_min(i)) f_pres = f_pres + q_cons_vf(i + contxb - 1)%sf(j, k, l)/rho_K_s(i) df_pres = df_pres - q_cons_vf(i + contxb - 1)%sf(j, k, l) & /(gs_min(i)*rho_K_s(i)*(pres_relax + ps_inf(i))) end if Potential Inconsistency Energetics mixing switches to gammas/pi_infs/qvs arrays; ensure array indices align with adv(l) composition and multi-fluid assumptions to avoid mis-weighting gamma, pi_inf, and qv in H and energy sums. do i = 0, m pres = 0._wp dV = dx(i)*dy(j)*dz(k) rho = 0._wp gamma = 0._wp pi_inf = 0._wp qv = 0._wp pres = q_prim_vf(E_idx)%sf(i, j, k) Egint = Egint + q_prim_vf(E_idx + 2)%sf(i, j, k)*(gammas(2)*pres)*dV do s = 1, num_vels vel(s) = q_prim_vf(num_fluids + s)%sf(i, j, k) Egk = Egk + 0.5_wp*q_prim_vf(E_idx + 2)%sf(i, j, k)*q_prim_vf(2)%sf(i, j, k)*vel(s)*vel(s)*dV Elk = Elk + 0.5_wp*q_prim_vf(E_idx + 1)%sf(i, j, k)*q_prim_vf(1)%sf(i, j, k)*vel(s)*vel(s)*dV if (abs(vel(s)) > maxvel) then maxvel = abs(vel(s)) end if end do do l = 1, adv_idx%end - E_idx adv(l) = q_prim_vf(E_idx + l)%sf(i, j, k) gamma = gamma + adv(l)*gammas(l) pi_inf = pi_inf + adv(l)*pi_infs(l) rho = rho + adv(l)*q_prim_vf(l)%sf(i, j, k) qv = qv + adv(l)*q_prim_vf(l)%sf(i, j, k)*qvs(l) end do H = ((gamma + 1._wp)*pres + pi_inf + qv)/rho

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

src/pre_process/m_assign_variables.fpp (1)

213-221: Tait exponent n_tait likely initialized from the wrong parameter

n_tait is now set from gs_min(1) and then transformed via n_tait = 1._wp/n_tait + 1._wp. This transformation appears to assume n_tait initially holds gamma, not “little gamma”/gs_min(1). If gs_min(1) is, as elsewhere, the precomputed lit_gamma, this change alters both the effective Tait exponent and the derived B_tait compared to the original formulation.

To preserve the original behavior while still using the conversion arrays, you likely want:

-        n_tait = gs_min(1)
-        B_tait = pi_infs(1)
+        n_tait = gammas(1)
+        B_tait = pi_infs(1)
 
         n_tait = 1._wp/n_tait + 1._wp
         B_tait = B_tait*(n_tait - 1._wp)/n_tait

Please confirm the intended meaning of gs_min(1) here and adjust accordingly so that the Tait parameters remain consistent with the rest of the EOS handling.

🧹 Nitpick comments (2)

src/pre_process/m_data_output.fpp (1)

276-279: Assignments to gamma, lit_gamma, pi_inf, and qv appear unused.

Given that gamma/pi_inf are immediately overwritten by s_convert_to_mixture_variables and lit_gamma is recomputed inside the 1D loop, the initial values from gammas(1), gs_min(1), pi_infs(1), and qvs(1) are never actually used. Consider dropping these four lines to reduce noise now that EOS parameters are fetched on demand.

src/simulation/m_data_output.fpp (1)

410-416: Several EOS scalars (gamma, lit_gamma, pi_inf, qv) are now written but never read.

In s_write_serial_data_files, gamma, lit_gamma, pi_inf, and qv are initialized from gammas(1), gs_min(1), pi_infs(1), and qvs(1) but not used later in the subroutine; similarly, lit_gamma assignments in s_write_probe_files are unused. They’re harmless, but you can safely remove these locals and their assignments now that the old fluid_pp lookups are gone.

Also applies to: 510-513, 1241-1243, 1355-1357

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11375f3 and 486d345.

📒 Files selected for processing (10)

• src/common/m_variables_conversion.fpp (3 hunks)
• src/post_process/m_data_output.fpp (3 hunks)
• src/post_process/m_derived_variables.fpp (1 hunks)
• src/pre_process/m_assign_variables.fpp (2 hunks)
• src/pre_process/m_data_output.fpp (1 hunks)
• src/pre_process/m_icpp_patches.fpp (6 hunks)
• src/simulation/m_data_output.fpp (3 hunks)
• src/simulation/m_global_parameters.fpp (0 hunks)
• src/simulation/m_pressure_relaxation.fpp (3 hunks)
• src/simulation/m_start_up.fpp (1 hunks)

💤 Files with no reviewable changes (1)

• src/simulation/m_global_parameters.fpp

🧰 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/post_process/m_derived_variables.fpp
• src/simulation/m_start_up.fpp
• src/pre_process/m_data_output.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.fpp
• src/common/m_variables_conversion.fpp
• src/simulation/m_data_output.fpp
• src/post_process/m_data_output.fpp
• src/pre_process/m_assign_variables.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/post_process/m_derived_variables.fpp
• src/simulation/m_start_up.fpp
• src/pre_process/m_data_output.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.fpp
• src/common/m_variables_conversion.fpp
• src/simulation/m_data_output.fpp
• src/post_process/m_data_output.fpp
• src/pre_process/m_assign_variables.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_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/simulation/m_data_output.fpp

🧠 Learnings (15)

📚 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:

• src/simulation/m_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.fpp
• src/common/m_variables_conversion.fpp
• 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} : Wrap tight loops with `$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')` macro; add `collapse=n` for safe nested loop merging

Applied to files:

• src/simulation/m_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.fpp
• src/common/m_variables_conversion.fpp
• 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_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.fpp
• src/common/m_variables_conversion.fpp
• src/pre_process/m_assign_variables.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:

• src/simulation/m_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• 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
• src/simulation/m_pressure_relaxation.fpp
• 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} : Do not use OpenACC or OpenMP directives directly; use Fypp macros from `src/common/include/parallel_macros.fpp` instead

Applied to files:

• src/simulation/m_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.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} : Ensure GPU code compiles with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort compilers

Applied to files:

• src/simulation/m_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.fpp
• 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} : Avoid stop/error stop inside GPU device code

Applied to files:

• src/simulation/m_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• 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 **/*.{fpp,f90} : Favor array operations over explicit loops when possible; use `collapse=N` directive to optimize nested loops

Applied to files:

• 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_start_up.fpp
• src/simulation/m_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.fpp
• 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 **/*.{fpp,f90} : Use `wp` (working precision) parameter from `m_precision_select` instead of hardcoded precision like `real*8`

Applied to files:

• src/pre_process/m_data_output.fpp
• src/pre_process/m_icpp_patches.fpp
• src/simulation/m_data_output.fpp
• src/pre_process/m_assign_variables.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_pressure_relaxation.fpp
• src/pre_process/m_icpp_patches.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 lower-case keywords and intrinsics (`do`, `end subroutine`, etc.)

Applied to files:

• src/pre_process/m_icpp_patches.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} : Keep private helpers in the module; avoid nested procedures

Applied to files:

• src/pre_process/m_icpp_patches.fpp
• src/post_process/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 **/*.{fpp,f90} : Each variable should have one clear purpose; do not use the same variable for multiple purposes

Applied to files:

• src/pre_process/m_assign_variables.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). (13)

• GitHub Check: Detect File Changes
• GitHub Check: Detect File Changes
• GitHub Check: Github (ubuntu, mpi, no-debug, false)
• GitHub Check: Github (macos, mpi, no-debug, false)
• GitHub Check: Github (macos, mpi, debug, false)
• GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
• GitHub Check: Github (ubuntu, mpi, no-debug, true)
• GitHub Check: Github (ubuntu, mpi, debug, true)
• GitHub Check: Github (ubuntu, mpi, debug, false)
• GitHub Check: Code Cleanliness Check
• GitHub Check: Coverage Test on CodeCov
• GitHub Check: cubic · AI code reviewer
• GitHub Check: Build & Publish

🔇 Additional comments (9)

src/simulation/m_pressure_relaxation.fpp (1)

159-196: Use of pi_infs, ps_inf, and gs_min in Newton solver looks consistent; please re‑check the pressure floor constant.

The replacement of fluid_pp/module‑local EOS storage with pi_infs(i), ps_inf(i), and gs_min(i) in pres_K_init, rho_K_s, and df_pres is algebraically consistent with the stiffened‑gas form and the later (pres_relax + ps_inf(i)) factors. The only thing I’d ask you to double‑check is the bound in Line [179]–[180]: pres_relax is clamped to -(1._wp - 1.e-8_wp)*ps_inf(i) + 1._wp whereas pres_K_init uses + 1.e-8_wp; if these were meant to be identical floors, you may want to harmonize them.

src/common/m_variables_conversion.fpp (2)

1309-1328: Energy and per‑fluid internal energy now consistent with s_compute_pressure and shared EOS arrays.

The updated expressions for q_cons_vf(E_idx)%sf and the model_eqns==3 internal energies correctly switch from fluid_pp(i)%{gamma,pi_inf,qv} to gammas(i), pi_infs(i), and qvs(i), and the non‑bubbles energy formula gamma*p + dyn_pres + pi_inf + qv now exactly inverts the (energy - dyn_p - pi_inf - qv)/gamma branch in s_compute_pressure. Indices (intxb/advxb/contxb) are preserved, so this looks like a purely data‑source refactor.

---

1627-1639: Sound‑speed mix rule correctly migrated to gammas, pi_infs, and gs_min.

In s_compute_speed_of_sound, both the alt_soundspeed two‑fluid bulk‑modulus computation and the model_eqns==3 mixture branch now pull EOS parameters from gammas, pi_infs, and gs_min instead of fluid_pp, without changing the underlying formulas. This keeps all EOS data centralized while preserving previous behavior.

src/post_process/m_derived_variables.fpp (1)

191-213: Alt‑sound‑speed path cleanly switched to shared EOS arrays.

The blkmod1/blkmod2 computation in s_derive_sound_speed now uses gammas(1:2) and pi_infs(1:2), aligning post‑process sound‑speed evaluation with the centralized EOS arrays. As long as s_initialize_variables_conversion_module (or equivalent) runs before any calls into this routine, this looks good.

src/simulation/m_start_up.fpp (1)

1054-1056: Internal energy update correctly switched to per-fluid arrays

The new RHS for v_vf(i + intxb - 1) uses gammas(i), pi_infs(i), and qvs(i) with the same indexing pattern as before, so the change is a straightforward replacement of fluid_pp(i)%... without altering loop structure or offsets.

Please double-check at runtime (e.g., with a small test) that gammas, pi_infs, and qvs are initialized for all i = 1..num_fluids before s_initialize_internal_energy_equations is called, to avoid silent misuse of default values.

src/post_process/m_data_output.fpp (1)

24-25: Energy diagnostics now consistently use conversion-module EOS arrays

Importing m_variables_conversion and replacing fluid_pp(l)%gamma/pi_inf/qv with gammas(l), pi_infs(l), and qvs(l) in s_write_energy_data_file keeps the structure of the energy and speed-of-sound calculations intact, while centralizing EOS data in the new arrays.

Please confirm that:

• gammas, pi_infs, and qvs are initialized in the post-process path before s_write_energy_data_file runs, and
• gammas(2) / q_prim_vf(E_idx + 2) indeed correspond to the phase whose internal energy you intend to track in Egint.

Also applies to: 1617-1632

src/pre_process/m_assign_variables.fpp (1)

566-573: Model 4 Tait EOS density now correctly sourced from EOS arrays

For the model_eqns == 4 branch, pi_inf, gamma, and lit_gamma are now taken from pi_infs(1), gammas(1), and gs_min(1), and the density is recomputed via the existing Tait relation using 1/lit_gamma. This matches the usage pattern in other modules that switched from fluid_pp(1)%... to the conversion arrays and keeps the formula structurally unchanged.

src/pre_process/m_icpp_patches.fpp (2)

34-35: Consistent use of conversion-module EOS arrays in patch helpers

Across the various ICPP patch routines, pi_inf, gamma, and lit_gamma are now initialized from pi_infs(1), gammas(1), and gs_min(1) after adding use m_variables_conversion. This mirrors the prior use of fluid_pp(1)%... while centralizing EOS data in the new arrays, without altering geometry logic or patch-selection behavior.

Also applies to: 215-218, 753-756, 916-919, 1002-1005

---

798-803: Density reinit for model 4 rectangle patch drops rhoref factor—verify intent

When q_prim_vf(1)%sf(i, j, 0) < 1.e-10 and model_eqns == 4, the rectangle patch now recomputes density via Tait EOS as:

q_prim_vf(1)%sf(i, j, 0) = (((q_prim_vf(E_idx)%sf(i, j, 0) + pi_inf) / (pref + pi_inf))**(1._wp/lit_gamma)) * &
                           (1._wp - q_prim_vf(alf_idx)%sf(i, j, 0))

without a rhoref factor, whereas the similar formula in s_assign_patch_species_primitive_variables still multiplies by rhoref.

Please confirm whether omitting rhoref here is intentional (e.g., due to a different normalization choice for this special patch) or whether the two Tait-based density resets should be kept consistent.

[cubic-dev-ai]

3 issues found across 10 files

Prompt for AI agents (all 3 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/pre_process/m_icpp_patches.fpp">

<violation number="1" location="src/pre_process/m_icpp_patches.fpp:802">
Zero-density fixup now drops the `rhoref` factor, producing densities off by the reference density whenever this path executes.</violation>
</file>

<file name="src/pre_process/m_assign_variables.fpp">

<violation number="1" location="src/pre_process/m_assign_variables.fpp:216">
Initialize n_tait with gammas(1) so the existing transformation still yields the correct Tait exponent (1+γ)/γ.</violation>
</file>

<file name="src/simulation/m_pressure_relaxation.fpp">

<violation number="1" location="src/simulation/m_pressure_relaxation.fpp:180">
Lower-bounding `pres_relax` with a `+ 1._wp` shift raises the enforced minimum by an entire pressure unit, sabotaging the consistency with the initialization clamp and producing artificially high partial pressures. Use the same 1e-8 offset that was applied when capping `pres_K_init` to keep the clamp near the physically admissible minimum.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

[codeant-ai]

CodeAnt AI finished reviewing your PR.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/pre_process/m_assign_variables.fpp (1)

216-216: Incorrect EOS parameter: use gammas(1) instead of gs_min(1) for Tait exponent.

The Tait exponent formula at line 248 uses 1/n_tait, expecting n_tait to be γ. Based on the past review and the formula ((1 + B_tait)/(p + B_tait))^(1/n_tait), n_tait should be initialized with the specific heat ratio γ, not the Grüneisen parameter (1+γ)/γ.

Apply this diff:

-        n_tait = gs_min(1)
+        n_tait = gammas(1)

🧹 Nitpick comments (1)

src/pre_process/m_assign_variables.fpp (1)

563-565: Consider computing lit_gamma directly for clarity.

Since gamma is already assigned on line 564, computing lit_gamma = (1._wp + gamma)/gamma directly would make the relationship explicit and reduce dependency on the pre-computed gs_min array. This improves code clarity and makes the Tait EOS formula self-documenting.

Apply this diff:

         pi_inf = pi_infs(1)
         gamma = gammas(1)
-        lit_gamma = gs_min(1)
+        lit_gamma = (1._wp + gamma)/gamma

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e05105 and 151504e.

📒 Files selected for processing (1)

• src/pre_process/m_assign_variables.fpp (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{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/pre_process/m_assign_variables.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/pre_process/m_assign_variables.fpp

🧠 Learnings (1)

📚 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/pre_process/m_assign_variables.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). (9)

• GitHub Check: Code Cleanliness Check
• GitHub Check: Coverage Test on CodeCov
• GitHub Check: Github (ubuntu, no-mpi, single, no-debug, false)
• GitHub Check: Github (ubuntu, mpi, no-debug, false)
• GitHub Check: Github (ubuntu, mpi, no-debug, true)
• GitHub Check: Github (ubuntu, mpi, debug, false)
• GitHub Check: Github (ubuntu, mpi, debug, true)
• GitHub Check: cubic · AI code reviewer
• GitHub Check: Build & Publish

🔇 Additional comments (1)

src/pre_process/m_assign_variables.fpp (1)

216-217: I'll verify the definitions and purpose of these variables by searching the codebase. Let me start by examining the definitions in m_variables_conversion.fpp.
<function_calls>

#!/bin/bash

First, locate the m_variables_conversion.fpp file

echo "=== Locating m_variables_conversion.fpp ==="
find . -name "m_variables_conversion.fpp" -type f

echo ""
echo "=== Searching for gs_min, ps_inf, gammas, pi_infs definitions ==="

Search in common/m_variables_conversion.fpp for these variable declarations

rg -n -A 3 -B 2 "gs_min|ps_inf|gammas|pi_infs" src/common/m_variables_conversion.fpp 2>/dev/null | head -150


</function_calls>

Wait, let me refine this search to be more thorough and check the entire codebase structure first:
<function_calls>

#!/bin/bash

Locate the file and search for variable definitions

echo "=== Finding m_variables_conversion.fpp ==="
fd "m_variables_conversion" --type f

echo ""
echo "=== Searching for array declarations with gs_min, ps_inf, gammas, pi_infs ==="
rg -n "gs_min|ps_inf" src/ --type fpp --type f90 -B 3 -A 3 | head -200


</function_calls>

Let me use a more targeted approach:
<function_calls>

#!/bin/bash

Find and examine m_variables_conversion.fpp

echo "=== Locating variables conversion file ==="
find src -name "variablesconversion*" -type f

echo ""
echo "=== Searching for gs_min definition ==="
rg -n "gs_min" src/ -B 2 -A 2

echo ""
echo "=== Searching for ps_inf definition ==="
rg -n "ps_inf" src/ -B 2 -A 2


</function_calls>

Let me search more broadly:
<function_calls>

#!/bin/bash

List all files in src/common to find the right file

echo "=== Files in src/common ==="
ls -la src/common/ | grep -E "variables|conversion"

echo ""
echo "=== Searching all .fpp files for gs_min ==="
find src -name "*.fpp" -exec rg -l "gs_min" {} ;

echo ""
echo "=== Searching all .fpp files for ps_inf ==="
find src -name "*.fpp" -exec rg -l "ps_inf" {} ;


</function_calls>

Let me try a more direct search:
<function_calls>

#!/bin/bash

Search for these variables in the entire src directory

echo "=== All occurrences of gs_min ==="
rg -rn "gs_min" src/ 2>/dev/null

echo ""
echo "=== All occurrences of ps_inf ==="
rg -rn "ps_inf" src/ 2>/dev/null

echo ""
echo "=== All occurrences of gammas ==="
rg -rn "gammas" src/ 2>/dev/null | head -50

echo ""
echo "=== All occurrences of pi_infs ==="
rg -rn "pi_infs" src/ 2>/dev/null | head -50


</function_calls>

[codecov]

Codecov Report

❌ Patch coverage is 8.69565% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.33%. Comparing base (11375f3) to head (41db1d9).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/pre_process/m_icpp_patches.fpp	0.00%	6 Missing ⚠️
src/common/m_variables_conversion.fpp	0.00%	4 Missing ⚠️
src/post_process/m_data_output.fpp	0.00%	4 Missing ⚠️
src/post_process/m_derived_variables.fpp	0.00%	4 Missing ⚠️
src/simulation/m_start_up.fpp	0.00%	2 Missing ⚠️
src/pre_process/m_assign_variables.fpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1063      +/-   ##
==========================================
- Coverage   44.35%   44.33%   -0.03%     
==========================================
  Files          71       71              
  Lines       20587    20580       -7     
  Branches     1993     1992       -1     
==========================================
- Hits         9132     9124       -8     
- Misses      10310    10312       +2     
+ Partials     1145     1144       -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.