Skip to content

Conversation

@JRChreim
Copy link
Contributor

@JRChreim JRChreim commented Nov 25, 2025

User description

User description

Description

This PR comprises the addition of the individual internal energies for N fluids when the 6-equation model is used and the conservative variables are output. It facilitates data analysis when this variable is needed.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

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.

How Has This Been Tested?

  • Any simulation that uses the 6-equation model and outputs the conservative variables. This can be seen on VisIt/ParaView. The output of the initial condition was compared against results by hand, resulting in the appropriate values. The figure below illustrates the output for the droplet aerobreakup problem
Screenshot from 2025-11-24 16-12-48

Test Configuration:

  • What computers and compilers did you use to test this:

Checklist

  • I have added comments for the new code
  • 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

PR Type

Enhancement

Description

  • Add individual internal energies output for 6-equation model

  • Output conservative variables for each fluid phase

  • Enable energy analysis in post-processing visualization

Diagram Walkthrough

flowchart LR
  A["6-equation model<br/>cons_vars_wrt enabled"] -->|Loop over fluids| B["Extract internal energy<br/>from q_cons_vf"]
  B -->|Format variable name| C["Write alpha_rho_e_i<br/>to database file"]
  C -->|Repeat for each fluid| D["Complete energy output"]
Loading

File Walkthrough

Relevant files
Enhancement
m_start_up.fpp
Add internal energies output loop for 6-equation model     

src/post_process/m_start_up.fpp

  • Added conditional block to output individual internal energies when
    6-equation model is active
  • Loops through all fluid phases to extract and write internal energy
    variables
  • Extracts internal energy from q_cons_vf array with proper indexing and
    offset handling
  • Writes formatted output with variable names alpha_rho_e_i for each
    fluid phase
+15/-0   

CodeAnt-AI Description

Expose per-fluid internal energies when exporting six-equation conservative data

What Changed

  • When conservative variables are written for the six-equation model, each fluid's internal energy is now emitted whenever its new alpha_rho_e flag or the main conservative flag is enabled.
  • The new flag can be set via post_process inputs, is initialized/broadcast across ranks, and is included in the formatted database configuration so visualization tools see the extra fields.
  • The start-up logic now reads and honors these flags, ensuring MPI runs produce the same dataset on every rank.

Impact

✅ Enables per-fluid internal energy visualization
✅ More comprehensive six-equation conservative dumps
✅ MPI runs respect new per-fluid energy requests

💡 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

  • New Features

    • Output now includes per-fluid energy contributions when using the multi-fluid model with conservative-variable output enabled, giving detailed energy breakdowns per fluid.
  • Chores

    • Added a new per-fluid output flag (configurable) and updated configuration templates to expose it.
    • Ensured the new flag is propagated in parallel runs so outputs remain consistent across processes.

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

@JRChreim JRChreim requested a review from a team November 25, 2025 00:26
@codeant-ai
Copy link

codeant-ai bot commented Nov 25, 2025

CodeAnt AI is reviewing your PR.

@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The variable naming omits the underscore shown in the description/diagram. Code writes 'alpha_rho_ei' (from 'alpha_rho_e' + I0) instead of the documented 'alpha_rho_e_i'. Verify intended naming for downstream tools and consistency with other outputs.

write (varname, '(A,I0)') 'alpha_rho_e', i
call s_write_variable_to_formatted_database_file(varname, t_step)
Indexing Risk

Directly indexing q_cons_vf(i + intxb - 1)%sf with extended offsets differs from nearby pattern (e.g., E_idx uses bounded slices x_beg:x_end,...). Validate that the offsets and bounds match q_sf dimensions and won’t read beyond halo or miss the intended subdomain.

q_sf = q_cons_vf(i + intxb - 1)%sf(-offset_x%beg:m + offset_x%end, &
                                   -offset_y%beg:n + offset_y%end, &
                                   -offset_z%beg:p + offset_z%end)
DRY/Reuse

The loop duplicates the variable-write pattern used above for conservative fields. Consider extracting a small helper to reduce repetition and ensure consistent handling of varname clearing and q_sf slicing.

if (model_eqns == 3 .and. cons_vars_wrt) then
    do i = 1, num_fluids

        q_sf = q_cons_vf(i + intxb - 1)%sf(-offset_x%beg:m + offset_x%end, &
                                           -offset_y%beg:n + offset_y%end, &
                                           -offset_z%beg:p + offset_z%end)

        write (varname, '(A,I0)') 'alpha_rho_e', i
        call s_write_variable_to_formatted_database_file(varname, t_step)

        varname(:) = ' '
    end do
end if

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 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 a per‑fluid energy output flag through the namelist, exposes the flag in global parameters and MPI broadcasts, and writes per‑fluid energy (alpha_rho_e) records in s_save_data when the multi‑component model is active and conservative variable output is enabled.

Changes

Cohort / File(s) Summary
Post-process startup
src/post_process/m_start_up.fpp
Added alpha_rho_e_wrt to the input namelist and appended a conditional block in s_save_data that, when model_eqns == 3 and alpha_rho_e_wrt or cons_vars_wrt is true, loops over fluids and writes alpha_rho_e per fluid to the formatted database (after existing E energy output).
Global parameters
src/post_process/m_global_parameters.fpp
Declared new public logical array alpha_rho_e_wrt(num_fluids_max), initialized it to .false. with other per‑fluid write flags, and included it among writable flags.
MPI synchronization
src/post_process/m_mpi_proxy.fpp
Broadcasts alpha_rho_e_wrt(1) in s_mpi_bcast_user_inputs so the per‑fluid write flag is synchronized across MPI ranks.
Tooling / runtime config
toolchain/mfc/run/case_dicts.py
Added alpha_rho_e_wrt (type LOG) to the POST_PROCESS config entries generated per fluid id.

Sequence Diagram(s)

sequenceDiagram
    participant User as User / Namelist
    participant Startup as s_read_input_file
    participant Globals as m_global_parameters
    participant MPI as s_mpi_bcast_user_inputs
    participant Save as s_save_data
    note over Startup,Globals: Input parsing & flag registration
    User->>Startup: provide alpha_rho_e_wrt in namelist
    Startup->>Globals: set alpha_rho_e_wrt(num_fluids)
    Globals->>MPI: expose alpha_rho_e_wrt for broadcast
    MPI->>MPI: broadcast alpha_rho_e_wrt to all ranks
    note over Save: Runtime output decision
    Save->>Save: if model_eqns==3 and (alpha_rho_e_wrt or cons_vars_wrt)
    Save->>Save: loop over fluids -> compute q_sf -> write `alpha_rho_e` per fluid
    Save-->>User: formatted database updated with per-fluid alpha_rho_e
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect s_save_data conditional placement and loop indices (q_cons_vf / intxb offsets).
  • Verify naming/format consistency for alpha_rho_e with existing output variables.
  • Confirm alpha_rho_e_wrt initialization and MPI broadcast cover full num_fluids range.
  • Check generated case_dicts.py entries align with tooling expectations.

Suggested labels

Review effort 3/5, size:M

Poem

🐰 A nibble of code, a hop and a hop,
Per‑fluid energies lined up on the top.
Flags whispered through ranks, neat and fleet,
Each alpha_rho_e now joins the sheet. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding internal energies to conservative variables output for the 6-equation model, which aligns with all file changes across the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description covers all essential sections: clear motivation, feature type, related scope, testing details with visualization, and completed checklist items.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@codeant-ai codeant-ai bot added the size:S This PR changes 10-29 lines, ignoring generated files label Nov 25, 2025
@codeant-ai
Copy link

codeant-ai bot commented Nov 25, 2025

CodeAnt AI finished reviewing your PR.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 76d6a5c and 8fc2f6c.

📒 Files selected for processing (1)
  • src/post_process/m_start_up.fpp (1 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/post_process/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/post_process/m_start_up.fpp
🪛 GitHub Actions: Lint Source
src/post_process/m_start_up.fpp

[error] 1-1: Process completed with exit code 1 during file inspection step: grep -iR -e '...' -e '---' -e '===' ./src/*

⏰ 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). (14)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Self Hosted (gpu, omp, frontier)
  • GitHub Check: Github (ubuntu, mpi, no-debug, false)
  • GitHub Check: Github (macos, mpi, debug, false)
  • GitHub Check: Self Hosted (gpu, acc, gt)
  • GitHub Check: Self Hosted (gpu, omp, gt)
  • GitHub Check: Github (macos, mpi, no-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: Build & Publish

@JRChreim JRChreim enabled auto-merge November 25, 2025 00:38
@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.36%. Comparing base (11375f3) to head (ed33c3c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/post_process/m_start_up.fpp 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1059   +/-   ##
=======================================
  Coverage   44.35%   44.36%           
=======================================
  Files          71       71           
  Lines       20587    20596    +9     
  Branches     1993     1995    +2     
=======================================
+ Hits         9132     9138    +6     
- Misses      10310    10312    +2     
- Partials     1145     1146    +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.

@JRChreim JRChreim disabled auto-merge November 25, 2025 17:24
@JRChreim JRChreim enabled auto-merge November 25, 2025 23:45
@JRChreim JRChreim disabled auto-merge November 25, 2025 23:45
@codeant-ai
Copy link

codeant-ai bot commented Nov 26, 2025

CodeAnt AI is running Incremental review

@codeant-ai codeant-ai bot added size:S This PR changes 10-29 lines, ignoring generated files and removed size:S This PR changes 10-29 lines, ignoring generated files labels Nov 26, 2025
@codeant-ai
Copy link

codeant-ai bot commented Nov 26, 2025

CodeAnt AI Incremental review completed.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 7 files (reviewed changes from recent commits).

Prompt for AI agents (all 1 issues)

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


<file name="src/post_process/m_start_up.fpp">

<violation number="1" location="src/post_process/m_start_up.fpp:426">
New alpha_rho_e fields are written to the formatted database, but dbvars is not incremented, so the binary header advertises too few variables and downstream readers desynchronize.</violation>
</file>

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

cubic-dev-ai bot pushed a commit that referenced this pull request Nov 26, 2025
@sbryngelson sbryngelson merged commit 364a237 into MFlowCode:master Nov 27, 2025
42 of 43 checks passed
@JRChreim JRChreim deleted the 6EqnEnergiesOutput branch November 27, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review effort 2/5 size:S This PR changes 10-29 lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

3 participants