Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .codeant/configuration.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"review_configuration": {
"include": [
"src/**/*.fpp",
"src/**/*.f90",
"toolchain/**/*.py"
],
"exclude": [
"tests/**",
"examples/**",
"docs/**"
]
}
}
27 changes: 27 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
language: en-US

reviews:
profile: chill
path_instructions:
- path: "src/**/*.fpp"
instructions: |
Fortran source (Fypp-preprocessed). Follow the coding standards in
docs/documentation/contributing.md and the GPU macro API in
docs/documentation/gpuParallelization.md.
- path: "src/**/*.f90"
instructions: |
Fortran source. Follow the coding standards in
docs/documentation/contributing.md.
- path: "toolchain/**/*.py"
instructions: |
Python toolchain code. Follow PEP 8. Requires Python 3.10+;
do not suggest __future__ imports or backwards-compatibility shims.

knowledge_base:
code_guidelines:
enabled: true
filePatterns:
- "docs/documentation/contributing.md"
- "docs/documentation/gpuParallelization.md"
- ".github/copilot-instructions.md"
118 changes: 10 additions & 108 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,113 +1,15 @@
# Contributing to the MFC Codebase (Multi‑Component Flow Code)
# Contributing to MFC

**Multi‑Component Flow Code (MFC)** is an open‑source, high‑performance code for simulating compressible multi‑component, multi‑phase flows.
We welcome contributions of all kinds—bug fixes, new features, documentation, tests, and issue triage—from both newcomers and experienced developers.
This guide explains how to set up your environment, follow MFC's coding standards, and navigate the pull-request (PR) process so your work can be merged smoothly.
We welcome contributions of all kinds -- bug fixes, new features, documentation, tests, and issue triage.

---
**Full developer guide:** [mflowcode.github.io/documentation/contributing.html](https://mflowcode.github.io/documentation/contributing.html)

## 1. Setting Up Your Development Environment
## Quick Reference

1. **Fork and clone**
```bash
git clone https://github.com/<your‑user>/MFC.git
cd MFC
git remote add upstream https://github.com/MFlowCode/MFC.git
```
2. **Build MFC** – follow the [documentation](https://mflowcode.github.io/documentation/md_getting-started.html). For example:
```bash
./mfc.sh build -j 8 # parallel build with 8 threads
```
3. **Run the test suite** to verify your environment:
```bash
./mfc.sh test -j 8
```

---

## 2. Development Workflow

| Step | Action | Notes |
|------|--------|-------|
| 1 | **Sync your fork**: `git checkout master && git pull upstream master` | Stay up‑to‑date to avoid merge conflicts. |
| 2 | **Create a branch**: `git checkout -b feature/<short‑name>` | Keep each branch focused on one logical change. |
| 3 | **Code, test, document** | Follow the guidelines in §3. |
| 4 | **Format & lint**: `./mfc.sh format` | CI will re‑check; make it pass locally first. |
| 5 | **Run tests**: `./mfc.sh test` | All existing and new tests must pass. |
| 6 | **Commit** (see *Commit Messages* below) | Write clear, atomic commits. |
| 7 | **Push** & open a **PR** | Be mindful: *every push triggers CI*. Bundle fixes together to avoid dozens of CI runs. |

### Commit Messages

- Start with a concise (≤50 chars) summary in imperative mood: `Fix out‑of‑bounds in EOS module`.
- Add a blank line, then a detailed explanation.
- Reference related issues or PRs, e.g., `Fixes #123`.

### Managing CI Runs

Each push to a branch with an open PR runs the full CI matrix (which can take hours).
Plan your pushes—run tests locally and group changes—so the CI queue is not flooded.

---

## 3. Coding Guidelines and Best Practices

### 3.1 Style, Formatting & Linting
MFC enforces a project‑wide Fortran style:
- **Formatter**: `./mfc.sh format` auto‑formats your changes.
- **Linter**: CI runs several linter checks that spot common Fortran-gotchas (implicit typing, shadowed variables, unused locals, etc.). Fix issues before pushing or the linter will often catch them.

### 3.2 Fypp Metaprogramming

MFC uses [**Fypp**](https://github.com/aradi/fypp), a lightweight Python-based preprocessor, to generate repetitive or accelerator-specific Fortran.
Key points:
- Fypp macros live in `include/` directories nested within `src/`.
- Run `./mfc.sh format` to format the example case files and the source code.
- When editing `.fpp`, maintain readability, prefer simple macros over deeply nested constructs.

### 3.3 Documentation

- Add or update Doxygen comments in source files.
- Update Markdown docs under `docs/` if user‑facing behavior changes.
- Provide a minimal example in `examples/` for each new feature when practical.

### 3.4 Testing

- Add regression tests that fail before your change and pass after.
- Use `./mfc.sh test --generate` to create golden files for new cases.
- Keep tests fast; favor small grids and short runtimes.

### 3.5 GPU & Performance

- Ensure code compiles for CPU *and* GPU targets (NVHPC for NVIDIA, Cray for AMD).
- Profile critical kernels; avoid introducing bottlenecks.

---

## 4. Preparing Your Pull Request

1. **One PR = One logical change**. If you plan a follow‑up change, open an issue describing it and assign yourself for visibility.
2. **Fill out the PR template**. Remove checkboxes that do **not** apply.
3. **Describe testing** – list commands, compilers, and any profiling.
4. **Link issues** – `Fixes #<id>` or `Part of #<id>`.
5. **Ensure CI passes** before requesting review.

> **Tip** If your change is large, consider splitting it into smaller PRs. Document the intent in an issue so reviewers understand the overall roadmap.

---

## 5. Code Review & Merge

- Respond promptly to reviewer comments.
- Push focused updates; each push re‑runs CI.
- When all reviews are approved and CI is green, a maintainer will merge your PR.

---

## 6. Issue Triage

If you prefer helping with issue management:
- Comment to clarify reproduction steps.
- Label issues when you have triage rights.
- Close fixed issues and reference the PR.
1. Fork and clone the repository
2. Create a feature branch: `git checkout -b feature/<short-name>`
3. Make your changes and run `./mfc.sh test`
4. Commit (formatting and linting run automatically via pre-commit hook)
5. Push and open a pull request

See the [developer guide](https://mflowcode.github.io/documentation/contributing.html) for coding standards, testing details, and the full PR process.
93 changes: 35 additions & 58 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -1,75 +1,52 @@
# GitHub Copilot Instructions – Pull-Request Reviews for MFC
# AI Code Review Instructions for MFC

These instructions guide **GitHub Copilot Code Review** and **Copilot Chat** when they evaluate pull requests in this repository.
These instructions guide AI code reviewers (GitHub Copilot, CodeRabbit, Qodo, Cubic, CodeAnt, etc.) when evaluating pull requests in this repository.

---

## 1 Project Context (always include)
**Coding standards, common pitfalls, and contribution guidelines:** `docs/documentation/contributing.md`
**GPU macro API reference:** `docs/documentation/gpuParallelization.md`

* **Project:** MFC (exascale many-physics solver) written in **modern Fortran 2008+**, generated with **Fypp**.
* **Directory layout:**
* Sources in `src/`, tests in `tests/`, examples in `examples/`.
* Most source files are templated `.fpp`; CMake transpiles them to `.f90`.
* **Fypp macros** are in `src/<subprogram>/include/`, where `<subprogram>` is `simulation`, `common`, `pre_process`, or `post_process`. Review these first.
* Only `simulation` (plus its `common` dependencies) is GPU-accelerated with **OpenACC**.

> **Copilot, when reviewing:**
> * Treat the codebase as free-form Fortran 2008+ with `implicit none`, explicit `intent`, and standard intrinsics.
> * Prefer `module … contains … subroutine foo()` over legacy constructs; flag uses of `COMMON`, file-level `include`, or other global state.
Formatting and linting are enforced by pre-commit hooks. Focus review effort on correctness, not style.

---

## 2 Incremental-Change Workflow
## 1 Project Context

Copilot, when reviewing:
* Encourage small, buildable commits
* **Project:** MFC (exascale many-physics solver) written in **modern Fortran 2008+**, preprocessed with **Fypp**.
* **Directory layout:**
* Sources in `src/`, tests in `tests/`, examples in `examples/`, Python toolchain in `toolchain/`.
* Most source files are `.fpp` (Fypp templates); CMake transpiles them to `.f90`.
* **Fypp macros** are in `src/<subprogram>/include/`, where `<subprogram>` is `simulation`, `common`, `pre_process`, or `post_process`.
* Only `simulation` (plus its `common` dependencies) is GPU-accelerated via **OpenACC**.
* Code must compile with **GNU gfortran**, **NVIDIA nvfortran**, **Cray ftn**, and **Intel ifx**.
* Precision modes: double (default), single, and mixed (`wp` = working precision, `stp` = storage precision).
* **Python toolchain** requires **Python 3.10+** — do not suggest `from __future__` imports or other backwards-compatibility shims.

---

## 3 Style & Naming Conventions (*.fpp / *.f90)
## 2 What to Review

| Element | Rule |
|---------|------|
| Indentation | 2 spaces; continuation lines align beneath &. |
| Case | Lower-case keywords & intrinsics (do, end subroutine, …). |
| Modules | m_<feature> (e.g. m_transport). |
| Public subroutines | s_<verb>_<noun> (s_compute_flux). |
| Public functions | f_<verb>_<noun>. |
| Routine size | subroutine ≤ 500 lines, helper ≤ 150, function ≤ 100, file ≤ 1000. |
| Arguments | ≤ 6; else use a derived-type params struct. |
| Forbidden | goto (except legacy), COMMON, save globals. |
| Variables | Every arg has explicit intent; use dimension/allocatable/pointer as appropriate. |
| Errors | Call s_mpi_abort(<msg>), never stop or error stop. |
See the **Common Pitfalls** section of `docs/documentation/contributing.md` for the full reference. Key review priorities:

Copilot, when reviewing:
* Flag violations of any cell above.
* Suggest refactors when size or argument limits are exceeded.
* Ensure private helpers stay inside their defining module and avoid nested procedures.
1. **Correctness over style** — logic bugs, numerical issues, array bounds (non-unity lower bounds with ghost cells)
2. **Precision mixing** — `stp` vs `wp` conversions, no double-precision intrinsics (`dsqrt`, `dble`, `real(8)`, etc.)
3. **Memory** — `@:ALLOCATE`/`@:DEALLOCATE` pairing, GPU pointer setup (`@:ACC_SETUP_VFs`/`@:ACC_SETUP_SFs`)
4. **MPI** — halo exchange pack/unpack offsets, `GPU_UPDATE` around MPI calls, buffer sizing
5. **GPU** — no raw OpenACC/OpenMP pragmas (use Fypp GPU macros), `private(...)` on loop-local variables, no `stop`/`error stop` in device code
6. **Physics** — pressure formula must match `model_eqns`, `case_validator.py` constraints for new parameters
7. **Compiler portability** — all four compilers, Fypp macros for GPU and CPU builds
8. **`src/common/` blast radius** — changes affect all three executables

---

## 4 OpenACC Guidelines (for GPU kernels)

Wrap tight loops:

```fortran
!$acc parallel loop gang vector default(present) reduction(...)
```

* Add collapse(n) when safe.
* Declare loop-local variables with private(...).
* Allocate large arrays with managed or move them into a persistent !$acc enter data region at start-up.
* Avoid stop/error stop inside device code.
* Code must compile with Cray ftn, NVIDIA nvfortran, GNU gfortran, and Intel ifx/ifort.

---
## 3 PR-Pattern Triggers

## 5 Review Checklist (what Copilot should verify)
Flag these patterns when reviewing a pull request:

1. Buildability: PR includes build instructions or CI passes the staged build.
2. Tests: Focused tests are added/updated.
3. Style compliance: All rules in §3 are satisfied.
4. Module hygiene: No new global state; proper namespacing.
5. Performance: GPU code follows §4; no large host/device transfers in hot loops.
6. Documentation: Updated in-code comments and, when needed, README or docs site.
7. Regressions: No changes to outputs of golden tests without clear justification.
* PR adds a parameter in `toolchain/mfc/params/definitions.py` but does not update the `namelist /user_inputs/` in `src/*/m_start_up.fpp` or declare it in `src/*/m_global_parameters.fpp`
* PR adds a parameter with cross-parameter constraints but does not add validation in `toolchain/mfc/case_validator.py`
* PR modifies files in `src/common/` but does not mention testing all three targets (pre_process, simulation, post_process)
* PR adds `@:ALLOCATE` calls without matching `@:DEALLOCATE` in the corresponding finalization subroutine
* PR renames or moves files referenced in `docs/documentation/contributing.md` or `docs/documentation/gpuParallelization.md` without updating those docs
* PR adds a new `.fpp` module missing `implicit none` or missing `private`/`public` declarations
* PR modifies MPI pack/unpack logic in one sweep direction without updating all directions
* PR adds a new physics feature or model without a corresponding regression test
62 changes: 18 additions & 44 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,58 +1,32 @@
## Description

Please include a summary of the changes and the related issue(s) if they exist.
Please also include relevant motivation and context.
Summarize your changes and the motivation behind them.

Fixes #(issue) [optional]
Fixes #(issue)

### Type of change

Please delete options that are not relevant.
- [ ] Bug fix
- [ ] New feature
- [ ] Refactor
- [ ] Documentation
- [ ] Other: _describe_

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Something else
## Testing

### Scope
How did you test your changes?

- [ ] 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?

Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce.
Please also list any relevant details for your test configuration
## Checklist

- [ ] Test A
- [ ] Test B
- [ ] I added or updated tests for new behavior
- [ ] I updated documentation if user-facing behavior changed

**Test Configuration**:
See the [developer guide](https://mflowcode.github.io/documentation/contributing.html) for full coding standards.

* What computers and compilers did you use to test this:
<details>
<summary><strong>GPU changes</strong> (expand if you modified <code>src/simulation/</code>)</summary>

## Checklist
- [ ] GPU results match CPU results
- [ ] Tested on NVIDIA GPU or AMD GPU

- [ ] I have added comments for the new code
- [ ] I added Doxygen docstrings to the new code
- [ ] I have made corresponding changes to the documentation (`docs/`)
- [ ] I have added regression tests to the test suite so that people can verify in the future that the feature is behaving as expected
- [ ] I have added example cases in `examples/` that demonstrate my new feature performing as expected.
They run to completion and demonstrate "interesting physics"
- [ ] 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](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) 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
</details>
15 changes: 14 additions & 1 deletion .pr_agent.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,20 @@ pr_commands = ["/describe", "/review", "/improve"]
num_max_findings = 10 # how many items to surface
require_tests_review = true
extra_instructions = """
Focus on duplicate code, the possibility of bugs, and if the PR added appropriate tests if it added a simulation feature.
Project context and review priorities: .github/copilot-instructions.md
Coding standards and common pitfalls: docs/documentation/contributing.md
GPU macro API: docs/documentation/gpuParallelization.md

Prioritize correctness over style (formatting is enforced by pre-commit hooks).
Key areas: logic bugs, numerical issues,
array bounds (non-unity lower bounds with ghost cells), MPI halo exchange
correctness (pack/unpack offsets, GPU data coherence), precision mixing
(stp vs wp), ALLOCATE/DEALLOCATE pairing (GPU memory leaks), physics model
consistency (pressure formula must match model_eqns), missing case_validator.py
constraints for new parameters, and compiler portability across all four
supported compilers.
Python toolchain requires Python 3.10+; do not suggest __future__ imports
or other backwards-compatibility shims.
"""

[pr_code_suggestions]
Expand Down
6 changes: 3 additions & 3 deletions docs/documentation/case.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,9 @@ end if

Some patch configurations are not adequately handled with the above analytic variable definitions.
In this case, a hard coded patch can be used.
Hard coded patches can be added by adding additional hard coded patch identifiers to `src/pre_process/include/1[2,3]dHardcodedIC.fpp`.
Hard coded patches can be added by adding additional hard coded patch identifiers to `src/common/include/1[2,3]dHardcodedIC.fpp`.
When using a hard coded patch, the `patch_icpp(patch_id)%%hcid` must be set to the hard-coded patch id.
For example, to add a 2D Hardcoded patch with an id of 200, one would add the following to `src/pre_process/include/2dHardcodedIC.fpp`
For example, to add a 2D Hardcoded patch with an id of 200, one would add the following to `src/common/include/2dHardcodedIC.fpp`

```f90
case(200)
Expand All @@ -256,7 +256,7 @@ The code provides three pre-built patches for dimensional extrusion of initial c
- `case(370)`: Extrude 2D data to 3D domain

Setup: Only requires specifying `init_dir` and filename pattern via `zeros_default`. Grid dimensions are automatically detected from the data files.
Implementation: All variables and file handling are managed in `src/pre_process/include/ExtrusionHardcodedIC.fpp` with no manual grid configuration needed.
Implementation: All variables and file handling are managed in `src/common/include/ExtrusionHardcodedIC.fpp` with no manual grid configuration needed.
Usage: Ideal for initializing simulations from lower-dimensional solutions, enabling users to add perturbations or modifications to the base extruded fields for flow instability studies.

#### Parameter Descriptions
Expand Down
Loading
Loading