Skip to content

Conversation

@danieljvickers
Copy link
Member

@danieljvickers danieljvickers commented Jan 21, 2026

User description

User description

User description

@sbryngelson this is a PR to check the benchmarks. I would appreciate it getting approved to run those.

Just for you, I also fixed the formatting check on master.


PR Type

Enhancement


Description

  • Remove extra blank lines in Python example files

  • Fix trailing whitespace in Fortran comment

  • Improve code formatting consistency across codebase


Diagram Walkthrough

flowchart LR
  A["Python Files<br/>examples/"] -->|"Remove blank lines"| B["Cleaner imports<br/>section"]
  C["Fortran File<br/>m_ibm.fpp"] -->|"Remove trailing<br/>whitespace"| D["Consistent<br/>formatting"]
  B --> E["Improved code<br/>quality"]
  D --> E
Loading

File Walkthrough

Relevant files
Formatting
m_ibm.fpp
Remove trailing whitespace from comment                                   

src/simulation/m_ibm.fpp

  • Remove trailing whitespace from comment on line 1183
  • Maintain code logic and functionality
+1/-1     
case.py
Remove extra blank line after imports                                       

examples/2D_kelvin_helmholtz/case.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
case.py
Remove extra blank line after imports                                       

examples/2D_richtmyer_meshkov/case.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
case.py
Remove extra blank line after imports                                       

examples/2D_viscous_shock_tube/case.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
analyze.py
Remove extra blank line after imports                                       

examples/nD_perfect_reactor/analyze.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
export.py
Remove extra blank line after imports                                       

examples/nD_perfect_reactor/export.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     


CodeAnt-AI Description

Normalize formatting in examples and fix minor spacing in simulation code

What Changed

  • Removed extraneous blank lines and trailing whitespace from multiple example Python scripts so they have consistent, compact formatting
  • Trimmed an extra trailing space in a Fortran comment near the moment-of-inertia fallback; no algorithm or numeric logic changed

Impact

✅ Fewer CI formatting failures
✅ Cleaner example scripts for readers and contributors
✅ No change to simulation behavior or output

💡 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

  • Chores
    • Removed several unused constants and top-level variable initializations in example and analysis scripts to simplify configuration.
    • Applied whitespace and formatting cleanups across example files for consistency.
    • Clarified an internal comment in simulation code to improve maintainability.

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


PR Type

Enhancement


Description

  • Remove extra blank lines after imports in Python example files

  • Improve Fortran comment clarity with additional context

  • Enhance code formatting consistency across codebase


Diagram Walkthrough

flowchart LR
  A["Python Examples<br/>5 files"] -->|"Remove blank lines"| B["Cleaner import<br/>sections"]
  C["Fortran File<br/>m_ibm.fpp"] -->|"Clarify comment"| D["Better code<br/>documentation"]
  B --> E["Improved code<br/>quality"]
  D --> E
Loading

File Walkthrough

Relevant files
Documentation
m_ibm.fpp
Clarify moment of inertia calculation comment                       

src/simulation/m_ibm.fpp

  • Enhance comment clarity by adding "via a sum" to explain the
    approximation method
  • Maintain code logic and functionality
+1/-1     
Formatting
case.py
Remove extra blank line after imports                                       

examples/2D_kelvin_helmholtz/case.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
case.py
Remove extra blank line after imports                                       

examples/2D_richtmyer_meshkov/case.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
case.py
Remove extra blank line after imports                                       

examples/2D_viscous_shock_tube/case.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
analyze.py
Remove extra blank line after imports                                       

examples/nD_perfect_reactor/analyze.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     
export.py
Remove extra blank line after imports                                       

examples/nD_perfect_reactor/export.py

  • Remove extra blank line after imports section
  • Improve code formatting consistency
+0/-1     

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Jan 21, 2026

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

Removes module-level constants/initializations (eps, mu, case) and trims blank lines across several example scripts; a minor comment change in src/simulation/m_ibm.fpp is also included.

Changes

Cohort / File(s) Summary
Constant removals
examples/2D_kelvin_helmholtz/case.py, examples/2D_viscous_shock_tube/case.py
Deleted module-level constants eps and mu respectively; references to these names may now be undefined.
Module variable removal
examples/nD_perfect_reactor/analyze.py
Removed module-scope initialization case = mfc.viz.Case(".", dt) (public case no longer initialized at import).
Whitespace normalization
examples/2D_richtmyer_meshkov/case.py, examples/nD_perfect_reactor/export.py
Removed blank lines between imports and subsequent assignments; no logic change.
Comment tweak
src/simulation/m_ibm.fpp
Minor comment wording change in s_compute_moment_of_inertia branch; no functional change.

Sequence Diagram(s)

(omitted — changes do not introduce a new multi-component control flow that requires visualization)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Three new examples cases #1060 — Modifications touching the same example files/constants (eps, mu, module-level case); changes appear directly related.

Suggested labels

size:M

Suggested reviewers

  • sbryngelson

Poem

🐰 I chewed a tiny var away,

blank lines hopped out to play,
comments nudged with gentle bite,
examples leaner, feeling light,
a happy nibble, day and night.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Pretty Comments' is vague and generic, failing to convey the actual changes made (removed blank lines and trailing whitespace formatting fixes). Use a more descriptive title such as 'Remove extra blank lines and trailing whitespace from examples and simulation code' to clearly communicate the formatting changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The PR description is well-structured with clear sections explaining the changes, motivation, and impacts. It includes a diagram and detailed file walkthrough. However, the checklist is not filled out according to the template requirements.
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

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

@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Formatting Only

The only functional-code-area change is whitespace in a comment; confirm no other trailing whitespace or formatting changes in this file were missed that could affect preprocessor directives or fixed-format assumptions in Fortran tooling.

    patch_ib(ib_marker)%moment = 0.0625_wp*patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%length_x**2 + patch_ib(ib_marker)%length_y**2)
elseif (patch_ib(ib_marker)%geometry == 8) then ! sphere
    patch_ib(ib_marker)%moment = 0.4*patch_ib(ib_marker)%mass*(patch_ib(ib_marker)%radius)**2

else ! we do not have an analytic moment of inertia calculation and need to approximate it directly 
    count = 0
    moment = 0._wp
    cell_volume = (x_cc(1) - x_cc(0))*(y_cc(1) - y_cc(0)) ! computed without grid stretching. Update in the loop to perform with stretching
Consistency

Multiple example scripts are making the same “remove extra blank line after imports” adjustment; consider enforcing this via a formatter/linter configuration to avoid repetitive formatting-only diffs across many files.

import cantera as ct
import seaborn as sns
from tqdm import tqdm
import matplotlib.pyplot as plt

import mfc.viz
from case import dt, Tend, SAVE_COUNT, sol

case = mfc.viz.Case(".", dt)

sns.set_theme(style=mfc.viz.generate_cpg_style())

@codeant-ai codeant-ai bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Jan 21, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 21, 2026

PR Code Suggestions ✨

No code suggestions found for the 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 6 files

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Jan 21, 2026

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Division by zero risk
    The code calculates the moment of inertia using patch_ib(ib_marker)%moment = moment*patch_ib(ib_marker)%mass/(count*cell_volume) after a parallel loop that increments count. If count remains zero (no grid cells found for the IB patch), this will divide by zero. Ensure there is a check for count > 0 and a defined fallback behavior.

  • Potential None formatting error
    find_induction_time returns None when no induction time is found, but later the return values are formatted with f"{ct_induction_time:.3e}" and similar. Formatting None with numeric format specifiers will raise a TypeError at runtime. Either ensure the function always returns a numeric sentinel (e.g. float('nan')), or guard/format the printed values when None is possible.

  • Timestamp / timestep mismatch
    The call to find_induction_time for MFC uses sorted(case.get_timestamps()) for ts but builds Ys and rhos from sorted(case.get_timesteps()). These may not align and will produce incorrect mapping between times and values. Use the same sequence (timesteps or timestamps derived consistently) for all three arguments.

  • Non-deterministic iteration
    The code builds Y_VARS as the union of two sets (Y_MAJORS | Y_MINORS) and then iterates over Y_VARS. Sets are unordered, so the iteration order can vary between runs/environments which may make loading variables, plotting order, and benchmarks non-deterministic and flaky. Consider using an ordered collection (sorted list or explicit list) to ensure deterministic behavior.

  • Non-deterministic CSV columns
    The header for "mfc.csv" is built using set(...) which does not preserve order. That produces a non-deterministic column order across runs and can break downstream comparisons or indexing assumptions. Use a deterministic ordering (preserve original order or sort the keys) when creating CSV headers.

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

🤖 Fix all issues with AI agents
In `@src/simulation/m_ibm.fpp`:
- Line 1182: Remove the trailing whitespace at the end of the comment line
containing "else ! we do not have an analytic moment of inertia calculation and
need to approximate it directly" in src/simulation/m_ibm.fpp so the formatter
passes; edit that comment line to have no trailing space after the word
"directly".

@codeant-ai
Copy link
Contributor

codeant-ai bot commented Jan 21, 2026

CodeAnt AI finished reviewing your PR.

sbryngelson
sbryngelson previously approved these changes Jan 21, 2026
@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.07%. Comparing base (cbcedd6) to head (a89fcdb).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1113   +/-   ##
=======================================
  Coverage   44.07%   44.07%           
=======================================
  Files          71       71           
  Lines       20369    20369           
  Branches     1986     1986           
=======================================
  Hits         8978     8978           
  Misses      10246    10246           
  Partials     1145     1145           

☔ 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 reopened this Jan 21, 2026
@sbryngelson sbryngelson merged commit 6264d3f into MFlowCode:master Jan 21, 2026
81 of 104 checks passed
@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review effort 1/5 size:XS This PR changes 0-9 lines, ignoring generated files

Development

Successfully merging this pull request may close these issues.

2 participants