Skip to content

Conversation

sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Jul 19, 2025

User description

Fixes typo fixes.


PR Type

Documentation


Description

  • Fixed typos and formatting inconsistencies throughout cursor rules

  • Corrected "Pogramming" to "Programming" in section header

  • Fixed "respository" to "repository" in documentation link

  • Added comprehensive new sections for file structure and Fortran practices


Diagram Walkthrough

flowchart LR
  A["Original Rules"] --> B["Typo Fixes"]
  B --> C["Formatting Cleanup"]
  C --> D["New Sections Added"]
  D --> E["Enhanced Documentation"]
Loading

File Walkthrough

Relevant files
Documentation
mfc-agent-rules.mdc
Comprehensive typo fixes and documentation expansion         

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

  • Fixed typos: "Pogramming" → "Programming", "respository" →
    "repository"
  • Cleaned up formatting and spacing inconsistencies
  • Added 6 new comprehensive sections covering file structure, Fypp
    macros, documentation style, error handling, memory management, and
    Fortran best practices
  • Enhanced GPU acceleration guidelines with more detailed examples
+122/-28

@Copilot Copilot AI review requested due to automatic review settings July 19, 2025 18:57
Copilot

This comment was marked as outdated.

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 FYPP macros and GPU acceleration with overlapping content. The GPU macro examples and guidelines appear in both sections, creating redundancy that could confuse users.

# 3 FYPP Macros for GPU acceleration Programming 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/958/files#diff-dfc3b3fa988ba58b3897926dc4334a9684383514b68556f28db8671337817b1aR149-R178'><strong>Inconsistent Formatting</strong></a>

The documentation uses inconsistent formatting patterns - some sections use bullet points with asterisks, others with dashes, and header numbering is inconsistent (Section 9 uses "9." while others use "#"). This reduces readability and professional appearance.
</summary>

```txt

- **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

Copy link

qodo-merge-pro bot commented Jul 19, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix YAML front matter syntax
Suggestion Impact:The suggestion was directly implemented - the YAML front matter was corrected from using `-----` delimiters and `-` prefixed keys to standard `---` delimiters and proper key-value pairs without hyphens

code diff:

-----
--description: Full MFC project rules – consolidated for Agent Mode
--alwaysApply: true
-----
+---
+description: Full MFC project rules – consolidated for Agent Mode
+alwaysApply: true
+---

The YAML front matter syntax is incorrect. YAML properties should not be
prefixed with hyphens when they are key-value pairs. The current format will
cause parsing errors.

.cursor/rules/mfc-agent-rules.mdc [1-4]

-----
--description: Full MFC project rules – consolidated for Agent Mode
--alwaysApply: true
-----
+---
+description: Full MFC project rules – consolidated for Agent Mode
+alwaysApply: true
+---
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the YAML front matter uses non-standard syntax (---- and - prefixes for keys) and proposes the standard --- delimiters and key-value pairs, which improves correctness and parser compatibility.

Medium
  • Update

sbryngelson and others added 3 commits July 19, 2025 14:59
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sbryngelson
Copy link
Member Author

/improve

@sbryngelson sbryngelson requested a review from Copilot July 19, 2025 19:06
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

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 corrects typos and formatting inconsistencies in the cursor rules documentation file, while also adding comprehensive new sections to enhance the MFC project's coding guidelines.

  • Fixed spelling errors including "Pogramming" → "Programming" and "respository" → "repository"
  • Cleaned up YAML front matter formatting and improved overall document structure
  • Added six new comprehensive sections covering file structure, Fypp macros, documentation style, error handling, memory management, and Fortran best practices

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sbryngelson sbryngelson merged commit e21901e into MFlowCode:master Jul 19, 2025
18 checks passed
@henryleberre
Copy link
Member

"Update cursor rules" is the new "Update README.md"

@sbryngelson
Copy link
Member Author

sbryngelson commented Jul 20, 2025

wait until you see the next generation of tom foolery @henryleberre

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.

2 participants