Skip to content

Conversation

sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Jul 19, 2025

User description

Adds a more full description of the cursor/AI rules (also helps just document our code practices). Related to #867


PR Type

Documentation


Description

  • Expanded Cursor AI rules with comprehensive Fortran/Fypp guidelines

  • Added detailed sections on file structure, GPU acceleration patterns

  • Enhanced documentation standards and error handling practices

  • Included memory management and code quality guidelines


Diagram Walkthrough

flowchart LR
  A["Basic Rules"] --> B["Comprehensive Guidelines"]
  B --> C["File Structure"]
  B --> D["GPU Acceleration"]
  B --> E["Documentation Standards"]
  B --> F["Error Handling"]
  B --> G["Memory Management"]
Loading

File Walkthrough

Relevant files
Documentation
mfc-agent-rules.mdc
Comprehensive expansion of MFC development guidelines       

.cursor/rules/mfc-agent-rules.mdc

  • Expanded from 3 to 10 sections with comprehensive development
    guidelines
  • Added detailed file structure, module layout, and naming conventions
  • Enhanced GPU acceleration documentation with Fypp macro examples
  • Added documentation style standards using Doxygen format
  • Included error handling, memory management, and code quality practices
+122/-28

@sbryngelson sbryngelson marked this pull request as ready for review July 19, 2025 18:51
@Copilot Copilot AI review requested due to automatic review settings July 19, 2025 18:51
@sbryngelson sbryngelson enabled auto-merge (squash) July 19, 2025 18:52
Copy link

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

Duplicate Content

Section 3 and Section 5 both cover GPU acceleration and Fypp macros with overlapping content. The GPU_PARALLEL_FOR macro is mentioned in both sections with similar explanations, creating redundancy that could confuse users.

# 3  FYPP Macros for GPU acceleration Pogramming Guidelines (for GPU kernels)

Do not directly use OpenACC or OpenMP directives directly.
Instead, use the FYPP macros contained in src/common/include/parallel_macros.fpp

Wrap tight loops with

```fortran
$:GPU_PARALLEL_FOR(private='[...]', copy='[...]')
  • Add collapse=n to merge nested loops when safe.
  • Declare loop-local variables with private='[...]'.
  • Allocate large arrays with managed or move them into a persistent
    $:GPU_ENTER_DATA(...) region at start-up.
  • Do not place stop / error stop inside device code.
  • Must compile with Cray ftn and NVIDIA nvfortran for GPU offloading; also build CPU-only with
    GNU gfortran and Intel ifx/ifort.

4 File & Module Structure

  • File Naming:

    • .fpp files: Fypp preprocessed files that get translated to .f90
    • Modules are named with m_ prefix followed by feature name: m_helper_basic, m_viscous
    • Primary program file is named p_main.fpp
  • Module Layout:

    • Start with Fypp include for macros: #:include 'macros.fpp'
    • Header comments using !> style documentation
    • module declaration with name matching filename
    • use statements for dependencies
    • implicit none statement
    • private declaration followed by explicit public exports
    • contains section
    • Implementation of subroutines and functions

5 Fypp Macros and GPU Acceleration

Use of Fypp

  • Fypp Directives:
    • Start with #: (e.g., #:include, #:def, #:enddef)
    • Macros defined in include/*.fpp files
    • Used for code generation, conditional compilation, and GPU offloading

Some examples

Documentation on how to use the Fypp macros for GPU offloading is available at https://mflowcode.github.io/documentation/md_gpuParallelization.html

Some examples include:

  • $:GPU_ROUTINE(parallelism='[seq]') - Marks GPU-callable routines
  • $:GPU_PARALLEL_LOOP(collapse=N) - Parallelizes loops
  • $:GPU_LOOP(parallelism='[seq]') - Marks sequential loops
  • $:GPU_UPDATE(device='[var1,var2]') - Updates device data
  • $:GPU_ENTER_DATA(copyin='[var]') - Copies data to device
  • $:GPU_EXIT_DATA(delete='[var]') - Removes data from device

</details>

<details><summary><a href='https://github.com/MFlowCode/MFC/pull/955/files#diff-dfc3b3fa988ba58b3897926dc4334a9684383514b68556f28db8671337817b1aR86-R178'><strong>Inconsistent Formatting</strong></a>

The documentation uses inconsistent formatting patterns - some sections use bullet points while others use numbered lists, and code block formatting varies between sections. This inconsistency could make the rules harder to follow.
</summary>

```txt

- **File Naming**:
  - `.fpp` files: Fypp preprocessed files that get translated to `.f90`
  - Modules are named with `m_` prefix followed by feature name: `m_helper_basic`, `m_viscous`
  - Primary program file is named `p_main.fpp`

- **Module Layout**:
  - Start with Fypp include for macros: `#:include 'macros.fpp'`
  - Header comments using `!>` style documentation
  - `module` declaration with name matching filename
  - `use` statements for dependencies
  - `implicit none` statement
  - `private` declaration followed by explicit `public` exports
  - `contains` section
  - Implementation of subroutines and functions

# 5  Fypp Macros and GPU Acceleration

## Use of Fypp
- **Fypp Directives**:
  - Start with `#:` (e.g., `#:include`, `#:def`, `#:enddef`)
  - Macros defined in `include/*.fpp` files
  - Used for code generation, conditional compilation, and GPU offloading

## Some examples

Documentation on how to use the Fypp macros for GPU offloading is available at https://mflowcode.github.io/documentation/md_gpuParallelization.html

Some examples include:
- `$:GPU_ROUTINE(parallelism='[seq]')` - Marks GPU-callable routines
- `$:GPU_PARALLEL_LOOP(collapse=N)` - Parallelizes loops
- `$:GPU_LOOP(parallelism='[seq]')` - Marks sequential loops
- `$:GPU_UPDATE(device='[var1,var2]')` - Updates device data
- `$:GPU_ENTER_DATA(copyin='[var]')` - Copies data to device
- `$:GPU_EXIT_DATA(delete='[var]')` - Removes data from device

# 6  Documentation Style

- **Subroutine/Function Documentation**:
  ```fortran
  !> This procedure <description>
  !! @param param_name Description of the parameter
  !! @return Description of the return value (for functions)

which conforms to the Doxygen Fortran format.

7 Error Handling

  • Assertions:

    • Use the fypp ASSERT macro for validating conditions
    • Example: @:ASSERT(predicate, message)
  • Error Reporting:

    • Use s_mpi_abort(<msg>) for error termination, not stop
    • No stop / error stop inside device code

8 Memory Management

  • Allocation/Deallocation:
    • Use fypp macro @:ALLOCATE(var1, var2) macro for device-aware allocation
    • Use fypp macro @:DEALLOCATE(var1, var2) macro for device-aware deallocation

9. Additional Observed Patterns

  • Derived Types:

    • Extensive use of derived types for encapsulation
    • Use pointers within derived types (e.g., pointer, dimension(:,:,:) => null())
    • Clear documentation of derived type components
  • Pure & Elemental Functions:

    • Use pure and elemental attributes for side-effect-free functions
    • Combine them for operations on arrays (pure elemental function)
  • Precision Handling:

    • Use wp (working precision) parameter from m_precision_select
    • Never hardcode precision with real*8 or similar
  • Loop Optimization:

    • Favor array operations over explicit loops when possible
    • Use collapse=N directive to optimize nested loops

10. Fortran Practices to Avoid

  • Fixed Format: Only free-form Fortran is used

    • No column-position dependent code
  • Older Intrinsics: Avoid outdated Fortran features like:

    • equivalence statements
    • data statements (use initialization expressions)
    • Character*N (use character(len=N) instead)
  • Using same variable for multiple purposes: Maintain single responsibility

    • Each variable should have one clear purpose

</details>

</td></tr>
</table>

@sbryngelson sbryngelson disabled auto-merge July 19, 2025 18:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates and significantly expands the cursor/AI agent rules documentation for the MFC project. The update transforms a concise reference into a comprehensive development guide that documents both AI interaction patterns and the project's coding practices.

  • Restructures and reformats the existing sections for better readability
  • Adds extensive new sections covering file structure, documentation style, error handling, and memory management
  • Expands GPU programming guidelines with specific Fypp macro examples and best practices

---

# 3 FYPP Macros for GPU acceleration Pogramming Guidelines (for kernels)
# 3 FYPP Macros for GPU acceleration Pogramming Guidelines (for GPU kernels)
Copy link
Preview

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

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

There is a spelling error in the heading: 'Pogramming' should be 'Programming'.

Suggested change
# 3 FYPP Macros for GPU acceleration Pogramming Guidelines (for GPU kernels)
# 3 FYPP Macros for GPU acceleration Programming Guidelines (for GPU kernels)

Copilot uses AI. Check for mistakes.

- Assume free-form Fortran 2008+, `implicit none`, explicit `intent`, and modern intrinsics.
- Prefer `module … contains … subroutine foo()`; avoid `COMMON` blocks and file-level `include` files.
- **Read the full codebase and docs *before* changing code.**
- Docs: <https://mflowcode.github.io/documentation/md_readme.html> and the respository root `README.md`.
Copy link
Preview

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

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

There is a spelling error: 'respository' should be 'repository'.

Suggested change
- Docs: <https://mflowcode.github.io/documentation/md_readme.html> and the respository root `README.md`.
- Docs: <https://mflowcode.github.io/documentation/md_readme.html> and the repository root `README.md`.

Copilot uses AI. Check for mistakes.

@sbryngelson sbryngelson merged commit e4d00c0 into MFlowCode:master Jul 19, 2025
18 checks passed
sbryngelson added a commit that referenced this pull request Jul 19, 2025
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Fix typo in section header

There is a typo in "Pogramming" which should be "Programming". This affects the
readability and professionalism of the documentation.

.cursor/rules/mfc-agent-rules.mdc [65]

-# 3  FYPP Macros for GPU acceleration Pogramming Guidelines (for GPU kernels)
+# 3  FYPP Macros for GPU acceleration Programming Guidelines (for GPU kernels)
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies and fixes a typo ("Pogramming") in a section header, which improves the professionalism and readability of the documentation.

Low
Clarify error message parameter format

The placeholder should be more descriptive to clarify that it expects a string
message parameter.

.cursor/rules/mfc-agent-rules.mdc [138-140]

 - **Error Reporting**:
-  - Use `s_mpi_abort(<msg>)` for error termination, not `stop`
+  - Use `s_mpi_abort(error_message)` for error termination, not `stop`
   - No `stop` / `error stop` inside device code
Suggestion importance[1-10]: 2

__

Why: The suggestion proposes changing the placeholder <msg> to error_message for clarity, but <msg> is a standard and clear convention for a placeholder, making the proposed improvement minimal and subjective.

Low
  • More

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

Successfully merging this pull request may close these issues.

1 participant