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
8 changes: 4 additions & 4 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
* New branches cannot be made on MFlowCode/MFC, they are made on forks
* PRs:
* made using AI tools like Claude Code and Codex should say so.
* made using AI tools like Claude Code and Codex should say so.
* are made from those MFC forks
* that change CFD result need verification PR is correct
* that change CFD results need verification that the PR is correct
* follow template
* that break a feature but promise a followup PR to fix it are rejected
* Commands:
Expand All @@ -11,5 +11,5 @@
* Programming and Design:
* New code should follow the DRY principle and also make side-effect code DRY as well
* Comments should be as short as possible without sacrificing value
* GPU macros should follow existing the source's GPU macro principles and patterns
* Functions/subroutines/modules shorter is better while being correctness, fast, and separating concerns
* GPU macros should follow the source's existing GPU macro principles and patterns
* Functions/subroutines/modules shorter is better while being correct, fast, and separating concerns
10 changes: 7 additions & 3 deletions .github/scripts/preflight.sh
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ fi
# microarchitecture dies with SIGILL, which would be reported as a bad node and
# get a perfectly healthy one excluded.
newest_syscheck() {
find "$@" -name syscheck -type f -printf '%T@ %p\n' 2>/dev/null \
| sort -rn | head -1 | cut -d' ' -f2-
# ls -t rather than find -printf: -printf is GNU-only, and on a BSD find it
# fails into 2>/dev/null, so discovery silently returns nothing and every
# probe is skipped as "no syscheck binary".
find "$@" -name syscheck -type f -exec ls -t {} + 2>/dev/null | head -1
}

syscheck_bin=$(newest_syscheck build/install -path "*${device}*")
Expand Down Expand Up @@ -132,7 +134,9 @@ run_probe() {
fi
}

run_probe "${launcher[@]}"
# ${arr[@]+"${arr[@]}"} rather than "${arr[@]}": under set -u, bash 3.2 (which is
# what macOS ships) treats an empty array expansion as an unbound variable.
run_probe ${launcher[@]+"${launcher[@]}"}

# If this launcher does not take the flags we added, drop them and probe again
# rather than reporting a verdict about the node. Otherwise a launcher that
Expand Down
20 changes: 13 additions & 7 deletions .github/workflows/homebrew-release.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
name: Update Homebrew Formula on Release

# Triggers when a new version tag is pushed
# Triggers when a release is published. Not `push: tags`, which GitHub skips
# when the tagged commit message carries [skip ci] -- that silently dropped the
# v5.6.0 and v5.7.0 formula updates, while docker.yml fired for both because a
# release event is not a push event.
on:
push:
tags:
- 'v*'
release:
types: [published]
pull_request:
branches: [master]
paths:
Expand All @@ -28,6 +30,9 @@ permissions:
jobs:
update-homebrew-tap:
name: Update homebrew-mfc tap
# A prerelease tag (v5.8.0-rc1) would fail the X.Y.Z check below, so skip it
# rather than reporting a red release job.
if: ${{ github.event_name != 'release' || !github.event.release.prerelease }}
runs-on: ubuntu-latest
environment:
name: homebrew
Expand All @@ -44,8 +49,9 @@ jobs:
VERSION="5.2.0"
echo "::notice::PR test mode - using version $VERSION"
else
# Extract version from tag (remove 'v' prefix)
VERSION="${GITHUB_REF#refs/tags/v}"
# Extract version from the released tag (remove 'v' prefix)
VERSION="${{ github.event.release.tag_name }}"
VERSION="${VERSION#v}"
fi

if [[ ! "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
Expand Down Expand Up @@ -97,7 +103,7 @@ jobs:
echo "- Update formula" >> $GITHUB_STEP_SUMMARY
echo "- Push to tap" >> $GITHUB_STEP_SUMMARY
echo "" >> $GITHUB_STEP_SUMMARY
echo "The full workflow will run when a \`v*\` tag is pushed after merge." >> $GITHUB_STEP_SUMMARY
echo "The full workflow will run when a release is published after merge." >> $GITHUB_STEP_SUMMARY

- name: Checkout homebrew-mfc tap
if: ${{ github.event_name != 'pull_request' }}
Expand Down
2 changes: 1 addition & 1 deletion docs/documentation/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ Both human reviewers and AI code reviewers reference this section.

### Parameter Plumbing

- **Derived-type parameters are not auto-broadcast.** `generated_bcast.fpp` covers namelist *scalars* only. Each derived type (`chem_params`, `lag_params`, `rburn`) needs a hand-written `_emit_<name>` in `toolchain/mfc/params/generators/fortran_gen.py` plus its call site in that generator's simulation branch, and, if it is read on device, an explicit `$:GPU_UPDATE(device='[name]')` in both the target's `m_global_parameters.fpp` and `src/simulation/m_start_up.fpp` — `GPU_DECLARE` alone does not make it device-resident. Regrouping existing scalars into a derived type silently drops their broadcast, leaving every non-root rank holding the `dflt_real` sentinel. Single-rank golden files cannot catch this, so pair such a change with a `ppn=2` test and confirm it fails without the emitter.
- **Derived-type parameters are not auto-broadcast.** `generated_bcast.fpp` covers namelist *scalars* only. Each derived type (`chem_params`, `lag_params`, `rburn`) needs a hand-written `_emit_<name>` in `toolchain/mfc/params/generators/fortran_gen.py` plus its call site in that generator's simulation branch, and, if it is read on device, an explicit ``$:GPU_UPDATE(device='[name]')`` in both the target's `m_global_parameters.fpp` and `src/simulation/m_start_up.fpp` — `GPU_DECLARE` alone does not make it device-resident. Regrouping existing scalars into a derived type silently drops their broadcast, leaving every non-root rank holding the `dflt_real` sentinel. Single-rank golden files cannot catch this, so pair such a change with a `ppn=2` test and confirm it fails without the emitter.
- **A `patch_ib` member that immersed-boundary ghost-point code reads must also be set in `s_add_cloud_particle`** (`src/simulation/m_particle_cloud.fpp`). `particle_cloud_ibs` is allocated without default initialization, and `s_reduce_ib_patch_array` copies the whole struct into `patch_ib`, overwriting the defaults assigned in `s_assign_default_values_to_user_inputs`. Anything left unset reaches the solver as uninitialized memory, and only where the allocation is not already zero-filled. A platform-only NaN is the signature of this class: a garbage `v_blow` once failed an AMD lane with `ICFL is NaN` while every NVIDIA lane and all local runs passed.
- **Runtime checks go where they run.** Shared constraints belong in `src/common/m_checker_common.fpp`, simulation-only ones in `src/simulation/m_checker.fpp`, and pre- and post-process ones in their own `m_checker.fpp`. Those two `s_check_inputs` are currently empty; that is still the correct home for their checks, not `m_checker_common`.
- **Analytic initial conditions are compiled into the binary** and their expressions are AST-validated at case load, so syntax errors and unknown variables surface immediately and by name. Each IC variable maps to an `eqn_idx` expression in `QPVF_IDX_VARS` (`toolchain/mfc/case.py`); adding a patch-settable conserved variable means updating that map and the Fortran `eqn_idx` builder together, because a mismatch is a silent wrong index.
Expand Down
4 changes: 2 additions & 2 deletions docs/documentation/gpuParallelization.md
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ while the host still registers it. The first launch aborts with
followed by a segmentation fault. Never place a GPU kernel inside a `block` construct;
hoist it into its own (module) subroutine with the locals passed as arguments.

## Silent-Failure Traps
## Silent-Failure Traps {#silent-failure-traps}

Every entry here was measured. They share a failure mode: the build stays green and the
answer is wrong, or one backend diverges from all the others.
Expand All @@ -874,7 +874,7 @@ answer is wrong, or one backend diverges from all the others.
always use `GPU_PARALLEL_LOOP` / `END_GPU_PARALLEL_LOOP`.
- **An array whose bound is a device global** (`dimension(num_fluids)`,
`dimension(num_species)`) may be passed to a device routine from a parallel-loop body,
but **not from inside another `GPU_ROUTINE(parallelism='[seq]')`**. Cray OpenACC rejects
but **not from inside another ``GPU_ROUTINE(parallelism='[seq]')``**. Cray OpenACC rejects
the second form with `ftn-7066 ... Global in accelerator routine without declare`, and
reports it at whatever line it gave up on: remove one trigger and the message walks
forward to the next call, so the reported line is not the cause. Only the plain lanes
Expand Down
2 changes: 1 addition & 1 deletion docs/documentation/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ If a trace is empty (that is, the empty string `""`), it will not appear in the

Finally, the case is appended to the `cases` list, which will be returned by the `list_cases` function.

### Selection and Execution Pitfalls
### Selection and Execution Pitfalls {#selection-and-execution-pitfalls}

Each of these fails quietly rather than loudly.

Expand Down
8 changes: 5 additions & 3 deletions toolchain/mfc/lint_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,10 @@ def check_device_routine_element_args(repo_root: Path) -> list[str]:
routine containing any `GPU_LOOP`, called with an array element as an actual argument, reads
the element as garbage and never writes it back. Either alone is fine, every `routine` level
is affected, and the loop counts when it sits in anything the routine calls. Copy the element
to a scalar before the call and receive results into a scalar. See
sbryngelson/compiler-bugs cce/acc-routine-element-by-reference.
to a scalar before the call and receive results into a scalar. See the Silent-Failure
Traps section of docs/documentation/gpuParallelization.md, MFC issue
https://github.com/MFlowCode/MFC/issues/1815, and the reproducer at
https://github.com/sbryngelson/compiler-bugs/tree/main/cce/acc-routine-element-by-reference.
"""
src_dir = repo_root / SRC_DIR
files = {src: src.read_text(encoding="utf-8").splitlines() for src in _fortran_fpp_files(src_dir)}
Expand Down Expand Up @@ -683,7 +685,7 @@ def check_device_routine_element_args(repo_root: Path) -> list[str]:
for arg in _split_top_level(stmt[m.end() : j - 1]):
e = _ELEMENT_ARG.match(arg)
if e and ":" not in arg and not _VALUE_CALL_NAMES.match(e.group(1)):
errors.append(f" {rel}:{line_no} `{arg}` into `{name}` (a device routine with a seq loop): pass a scalar, see sbryngelson/compiler-bugs cce/acc-routine-element-by-reference")
errors.append(f" {rel}:{line_no} `{arg}` into `{name}` (a device routine with a seq loop): pass a scalar, see docs/documentation/gpuParallelization.md (Silent-Failure Traps)")
return errors


Expand Down
2 changes: 1 addition & 1 deletion toolchain/mfc/test_preflight.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def workspace(tmp_path):
# stand in for a launcher the test meant to be absent.
sysbin = tmp_path / "sysbin"
sysbin.mkdir()
for tool in ("bash", "find", "head", "tail", "cat", "sed", "grep", "tr", "cut", "date", "mkdir", "mv", "rm", "hostname", "env", "sort", "wc", "dirname", "basename"):
for tool in ("bash", "find", "head", "tail", "cat", "sed", "grep", "tr", "cut", "date", "mkdir", "mv", "rm", "hostname", "env", "sort", "wc", "dirname", "basename", "ls"):
for root in ("/usr/bin", "/bin"):
src = Path(root) / tool
if src.exists():
Expand Down