Open
Conversation
added 11 commits
March 30, 2026 22:18
Contributor
Review Summary by QodoAdd premixed flame IC cases and refactor extrusion workflow with configurable paths
WalkthroughsDescription• Add two new hardcoded IC cases (271, 272) for premixed flame simulations - Case 271: Premixed flame vortex interaction with dual counter-rotating vortices - Case 272: Premixed flame instability with sinusoidal front perturbation • Refactor IC extrusion workflow to use configurable files_dir and file_extension parameters - Replace hardcoded path and filename pattern with user-configurable parameters - Improve MPI communication for new parameters via broadcast • Add three reference example cases with complete configuration files - 1D flamelet case with San Diego mechanism - 2D premixed flame vortex interaction case - 2D premixed flame Landau instability case • Update parameter definitions and test suite for new IC extrusion examples Diagramflowchart LR
A["Hardcoded IC Cases<br/>271, 272"] -->|"Vortex & Instability<br/>Perturbations"| B["2D Premixed<br/>Flame Simulations"]
C["Configurable<br/>files_dir & file_extension"] -->|"Replace hardcoded<br/>init_dir"| D["Flexible IC<br/>Extrusion"]
E["Reference Examples<br/>1D, 2D cases"] -->|"Complete setup<br/>with case.py"| F["User-friendly<br/>Workflow"]
D --> B
F --> B
File Changes1. src/common/include/2dHardcodedIC.fpp
|
Contributor
Code Review by Qodo
1. files_extension parameter typo
|
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a new 1D flamelet example case and updates related documentation. A 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.gitignore (1)
57-57: Redundant ignore pattern (no functional change).Line 57 is already covered by
examples/**/*.dat(Line 64), so this new rule is a no-op. Consider removing it to keep ignore rules minimal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 96117184-e389-41b7-b7bb-97731b0c2155
⛔ Files ignored due to path filters (42)
examples/1D_flamelet/IC/prim.1.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.10.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.11.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.12.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.13.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.14.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.2.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.3.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.4.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.5.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.6.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.7.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.8.00.000000.datis excluded by!**/*.datexamples/1D_flamelet/IC/prim.9.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.1.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.10.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.11.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.12.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.13.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.14.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.2.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.3.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.4.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.5.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.6.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.7.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.8.00.000000.datis excluded by!**/*.datexamples/2D_premixed_flame_vortex/IC/prim.9.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.1.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.10.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.11.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.12.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.13.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.14.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.2.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.3.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.4.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.5.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.6.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.7.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.8.00.000000.datis excluded by!**/*.datexamples/2D_premixed_landau_insta/IC/prim.9.00.000000.datis excluded by!**/*.dat
📒 Files selected for processing (17)
.gitignoredocs/documentation/case.mdexamples/1D_flamelet/case.pyexamples/1D_flamelet/sandiego.yamlexamples/2D_premixed_flame_vortex/case.pyexamples/2D_premixed_landau_insta/case.pysrc/common/include/2dHardcodedIC.fppsrc/common/include/ExtrusionHardcodedIC.fppsrc/pre_process/m_global_parameters.fppsrc/pre_process/m_mpi_proxy.fppsrc/pre_process/m_start_up.fpptests/0D1E22F3/golden-metadata.txttests/0D1E22F3/golden.txttoolchain/mfc/params/definitions.pytoolchain/mfc/params/descriptions.pytoolchain/mfc/params/namelist_parser.pytoolchain/mfc/test/cases.py
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1349 +/- ##
==========================================
- Coverage 64.88% 64.13% -0.75%
==========================================
Files 70 70
Lines 18249 18337 +88
Branches 1507 1511 +4
==========================================
- Hits 11841 11761 -80
- Misses 5446 5534 +88
- Partials 962 1042 +80 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Code Review by Qodo
|
| #:enddef | ||
|
|
||
| #:def HardcodedReadValues() | ||
| if (.not. files_loaded) then | ||
| max_files = merge(sys_size, sys_size - 1, num_dims == 1) | ||
| do f = 1, max_files | ||
| write (file_num_str, '(I0)') f | ||
| fileNames(f) = trim(init_dir) // "prim." // trim(file_num_str) // ".00." // zeros_default // ".dat" | ||
| fileNames(f) = trim(files_dir) // "/" // "prim." // trim(file_num_str) // ".00." // trim(file_extension) // ".dat" |
Contributor
There was a problem hiding this comment.
1. files_dir undefined in simulation 📘 Rule violation ≡ Correctness
ExtrusionHardcodedIC.fpp now references files_dir and file_extension, but the simulation-side m_global_parameters/namelist/MPI broadcast do not define or distribute these parameters. This can break simulation compilation or cause rank-to-rank mismatches at runtime when using extrusion-based ICs.
Agent Prompt
## Issue description
The new parameters `files_dir` and `file_extension` are used by code included in the simulation build (`ExtrusionHardcodedIC.fpp`), but they are only declared/bound/broadcast in `src/pre_process`. Simulation must also declare them in `m_global_parameters`, read them from `simulation.inp` via the `user_inputs` namelist, and broadcast them to all ranks.
## Issue Context
`src/simulation/m_ib_patches.fpp` includes `ExtrusionHardcodedIC.fpp`, which now constructs file names using `files_dir` and `file_extension`.
## Fix Focus Areas
- src/simulation/m_global_parameters.fpp[24-35]
- src/simulation/m_start_up.fpp[84-114]
- src/simulation/m_mpi_proxy.fpp[58-70]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| #:enddef | ||
|
|
||
| #:def HardcodedReadValues() | ||
| if (.not. files_loaded) then | ||
| max_files = merge(sys_size, sys_size - 1, num_dims == 1) | ||
| do f = 1, max_files | ||
| write (file_num_str, '(I0)') f | ||
| fileNames(f) = trim(init_dir) // "prim." // trim(file_num_str) // ".00." // zeros_default // ".dat" | ||
| fileNames(f) = trim(files_dir) // "/" // "prim." // trim(file_num_str) // ".00." // trim(file_extension) // ".dat" |
Contributor
There was a problem hiding this comment.
2. Extrusion reads rank-00 only 🐞 Bug ≡ Correctness
The extrusion reader hard-codes the middle filename field to .00. (prim.<var>.00.<ext>.dat), so it cannot read data produced with parallel I/O that is split across ranks (prim.<var>.<rank>.<tstep>.dat). This can lead to incomplete/incorrect extruded initial conditions or runtime failures when users follow the documented prim.XX.YY.file_extension.dat naming.
Agent Prompt
### Issue description
IC extrusion currently assumes the source data files have processor-rank field `YY=00` in `prim.XX.YY.<ext>.dat`. This prevents extruding from source datasets produced with MPI/parallel I/O where data is split across multiple `YY` files.
### Issue Context
- The repository writes files as `prim.<var>.<proc_rank>.<t_step>.dat`.
- Documentation states the format is `prim.XX.YY.file_extension.dat` without stating `YY` must be `00`.
### Fix Focus Areas
- src/common/include/ExtrusionHardcodedIC.fpp[59-67]
- src/pre_process/m_data_output.fpp[162-166]
- docs/documentation/case.md[259-267]
### Fix options (pick one)
1) **Implement support**: read and merge/sort data across all available `YY` partitions for each `XX` (most robust).
2) **Add a parameter**: e.g., `files_rank` (I2.2) and build filenames with that rank instead of hard-coding `00`.
3) **Clarify & enforce**: update docs to require `YY=00` / serial output, and add an explicit runtime check + clear abort message when expected files are missing.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Code Review by Qodo
|
Comment on lines
98
to
102
| ! Calculate offsets | ||
| domain_xstart = x_coords(1) | ||
| x_step = x_cc(1) - x_cc(0) | ||
| delta_x = merge(x_cc(0) - domain_xstart + x_step/2.0, x_cc(index_x) - domain_xstart + x_step/2.0, num_dims == 1) | ||
| delta_x = merge(x_cc(0) - domain_xstart, x_cc(index_x) - domain_xstart + x_step/2.0, num_dims == 1) | ||
| global_offset_x = nint(abs(delta_x)/x_step) |
Contributor
There was a problem hiding this comment.
1. 1d extrusion index shift 🐞 Bug ≡ Correctness
HardcodedReadValues() now computes global_offset_x for 1D using x_cc(0) - x_coords(1) without the half-cell correction, but x_coords comes from files written with x_cb coordinates. This makes global_offset_x round to 1 instead of 0 (via nint), shifting all 1D extruded IC samples by one index.
Agent Prompt
### Issue description
1D IC extrusion computes `global_offset_x` using `delta_x = x_cc(0) - x_coords(1)` (where `x_coords` is read from the first column of `prim.*.dat`). Because `prim.*` files write `x_cb(j)` as the coordinate while `x_cc` is cell-centered, this introduces a half-cell mismatch and `nint` rounds it to a full index shift.
### Issue Context
- `x_cc(0:m) = (x_cb(0:m) + x_cb(-1:(m-1)))/2` (cell centers)
- `prim.*` output uses `x_cb(j)` as the x-coordinate
- `HardcodedReadValues()` uses `global_offset_x = nint(abs(delta_x)/x_step)` and then indexes `stored_values` with `idx = i + 1 + global_offset_x`
### Fix Focus Areas
- src/common/include/ExtrusionHardcodedIC.fpp[98-103]
### Suggested fix
- For `num_dims == 1`, reintroduce the half-step correction when comparing `x_cc` to file coordinates, e.g.:
- `delta_x = x_cc(0) - domain_xstart + x_step/2.0_wp`
- Add a short comment explaining that the first column in `prim.*` is `x_cb` while values are cell-centered.
- (Optional but ideal) add a small regression test covering hcid=170 on a uniform grid that verifies no one-cell shift occurs when extruding from a serial `prim.*` output.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summarize your changes and the motivation behind them.
IC Extrusion: The workflow for extending Initial Conditions to higher dimensions has been simplified. The updated methodology is now more robust and user-friendly.
Documentation & Examples: To assist users with this new methodology, three new reference examples have been uploaded to the repository (one 1D case and two 2D cases)
Fixes #(issue)
Type of change
-[x] A current methodology being more user-friendly
Testing
How did you test your changes?
Flame_Vortex_Interaction.mp4
-[x] 3rd Test, 2D Premixed Flame with perturbed flame front
Flame_Insta.mp4
Checklist
See the developer guide for full coding standards.
GPU changes (expand if you modified
src/simulation/)AI code reviews
Reviews are not triggered automatically. To request a review, comment on the PR:
@coderabbitai review— incremental review (new changes only)@coderabbitai full review— full review from scratch/review— Qodo review/improve— Qodo code suggestions@claude full review— Claude full review (also triggers on PR open/reopen/ready)claude-full-review— Claude full review via label