1687 fix: Make OpenMP no_create a documented no-op instead of aborting fypp.

[engineer-scientist]

OMP_NOCREATE_STR called #:stop whenever no_create was set. GPU_PARALLEL, GPU_PARALLEL_LOOP and GPU_DATA expand both the ACC and OMP directive strings at fypp time and let the compile-time #if select one, so the abort fired on OpenACC-only builds too -- making a documented parameter unusable on every backend.

OpenMP has no no_create equivalent, and OMP_DEFAULT_STR already emits a present-by-default mapping, so emitting nothing is the closest safe behavior. Document the clause as OpenACC-only, matching the existing GPU_CACHE note.

Generated Fortran is byte-identical across all 109 .fpp files in the three targets: every call site passes no_create=None, which already took the empty-string path.

Closes #1687

Claude-Session: https://claude.ai/code/session_016xc51kikzrKdHDfXf8VV5p

Description

OMP_NOCREATE_STR called #:stop 'no_create is not supported yet' whenever no_create was non-None. Because GPU_PARALLEL, GPU_PARALLEL_LOOP and GPU_DATA build both the ACC and OMP directive strings with #:set and then let the compile-time #if defined(MFC_OpenACC) / #elif defined(MFC_OpenMP) pick one, that #:stop fired during fypp preprocessing on OpenACC-only builds too — making a documented public parameter unusable on every backend, with an error pointing into omp_macros.fpp rather than at the caller.
This takes option (1) from the issue: the OpenMP path becomes a documented no-op, and the working OpenACC implementation (GEN_NOCREATE_STR) stays reachable.
Worth noting for context: MFC_OpenACC / MFC_OpenMP are never passed to fypp — cmake/Fypp.cmake only defines MFC_<CompilerId>, MFC_<TARGET>, MFC_COMPILER, MFC_CASE_OPTIMIZATION and chemistry. So fypp cannot know which backend will survive, and expanding both is structural rather than incidental. The invariant that follows is that every clause helper must be evaluable for every build; a #:stop in one backend's helper is an unconditional abort, not a guard.
Emitting nothing is the closest safe behavior rather than a silent wrong answer: OpenACC's no_create asks for "use the device copy if present, otherwise the host copy, don't allocate", and OMP_DEFAULT_STR already emits defaultmap(present:allocatable) (Cray) / defaultmap(tofrom:...) — the present-by-default behavior is already ambient.
Also adds (OpenACC only) to the no_create row of the three parameter tables in gpuParallelization.md, matching the existing GPU_CACHE convention ("NOTE Does not do anything for OpenMP currently"). Without it, the fix trades a loud failure for a silent one.
The replacement comment is deliberately the same line count as the code it replaces, so fypp's # line markers don't shift and the generated Fortran stays byte-identical (see Testing).

Closes #1687.

Type of change (delete unused ones)

• Bug fix
• Documentation

Testing

1. The issue's reproduction case. Same file and flags as the issue, MFC_SIMULATION only, no OpenMP anywhere:

Before: EXIT=1, omp_macros.fpp:71: error: no_create is not supported yet [FyppStopRequest]
After:  EXIT=0, !$acc parallel loop gang vector default(present) private(i) no_create(a)

2. All three affected macros (no_create='[a]' on each):
   Macro OpenACC OpenMP
   GPU_PARALLEL !$acc parallel default(present) no_create(a) !$omp target teams defaultmap(tofrom:aggregate) …
GPU_DATA !$acc data no_create(a) !$omp target data
GPU_PARALLEL_LOOP !$acc parallel loop gang vector default(present) private(i) no_create(a) !$omp target teams loop … private(i)
3. Zero blast radius — full-tree expansion is byte-identical. Since this is a header included nearly everywhere, I expanded every .fpp in the tree twice (original vs. patched omp_macros.fpp), using each target's real generated includes and the exact flag set from cmake/Fypp.cmake, and diffed the generated Fortran:
   Target Files Result
   pre_process 29 byte-identical
   simulation 57 byte-identical
   post_process 23 byte-identical
   Also repeated for chemistry=True and for --case-optimization + chemistry=True — byte-identical in every configuration.
   (Only fypp's embedded absolute source path differs between the two checkouts; normalized away.) Expected, since every call site passes no_create=None and already took the #:set no_create_val = '' path — but demonstrated rather than asserted.
4. Standard workflow, gfortran 13.3.0, CPU, no MPI:
   ./mfc.sh format — 294 files unchanged
   ./mfc.sh precheck — 7/7 passed
   ./mfc.sh build — pre_process, simulation, post_process all build
   ./mfc.sh test — 609/611 pass. Two chemistry cases failed: 2D -> Chemistry -> Reacting Mixing Layer (C4EB58A8, tolerance) and 2D -> Chemistry -> Spatial Reacting Mixing Layer (56F8C4BC, pre_process exit 1). I don't believe these are mine, and I checked rather than assumed: both pass when run in isolation with this patch applied, and both pass on stock master. Point 3's expansion diff also holds for their exact build configuration — byte-identical under chemistry=True (29 + 57 files) and under --case-optimization + chemistry=True (39 files) — so the binaries under test are unchanged by this patch. 56F8C4BC's error is hcid=274 file has more lines than the grid: examples/2D_spatial_reacting_mixing_layer/IC/prim.1.00.000000.dat, i.e. a shared example input read against the harness's shrunken 31×31 grid, which reads like interference in the parallel full-suite run. Caveat: I did not re-run the full 41-minute suite on master to reproduce them there, so I'm flagging rather than claiming a known flake — happy to dig further if you'd like.
   I have no GPU or ACC-capable compiler, so the emitted no_create(...) syntax has not been compiled by an OpenACC compiler — though it is the same GEN_PARENTHESES_CLAUSE output as every other ACC clause here, and nothing in the tree passes no_create at runtime today.
   No test added. toolchain/mfc/test/cases.py generates end-to-end solver runs compared against golden files; there is no unit layer for macro expansion, and a no_create case would require inventing a solver call site that passes the parameter. Happy to add a fypp-expansion smoke test as new infrastructure if you'd like it, but that felt like its own PR.

Checklist

Check these like this [x] to indicate which of the below applies.

• I added or updated tests for new behavior
• I updated documentation if user-facing behavior changed

See the developer guide for full coding standards.

GPU changes (expand if you modified src/simulation/)

• GPU results match CPU results
• Tested on NVIDIA GPU or AMD GPU

---

Audit of the other clauses (per the issue's closing question)

You asked whether create, attach, deviceptr and present share the same trap. I checked every #:stop in the macro layer:

Clause	OpenMP helper	Aborts?	Verdict
create	OMP_CREATE_STR → map(always,alloc:)	No	Fine
present	OMP_PRESENT_STR → map(present,alloc:)	No	Fine
deviceptr	OMP_DEVICEPTR_STR → use_device_ptr(...)	No	Fine
attach	mapped in OMP_PARALLEL*	No	Semantics questioned by #1693: left alone
default='none'	OMP_DEFAULT_STR (omp_macros.fpp:36)	Yes	Same bug class
default is public and documented as accepting 'present' or 'none'; GEN_DEFAULT_STR handles 'none' correctly but OMP_DEFAULT_STR #:stops on it, so GPU_PARALLEL_LOOP(default='none') aborts fypp on an OpenACC-only build for exactly the same reason.
I did not fix it here. Unlike no_create, emitting nothing for default='none' would be a semantic change rather than a no-op — default(none) demands explicit data clauses, and silently dropping it would weaken a correctness check the caller asked for. Happy to open a separate issue, or to fold a fix in here if you'd prefer.
The remaining #:stops (shared_parallel_macros.fpp:32,34,92, omp_macros.fpp:262,319) all reject malformed input — unbalanced parentheses, a reduction list without an operator, a GPU_DATA wrapping no code. Those are backend-independent and correct as-is.

AI code reviews

Reviews are not retriggered automatically. To request a review, comment on the PR:

• @claude full review — Claude full review (also triggers on PR open/reopen/ready)
• Or add label claude-full-review — Claude full review via label

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.59%. Comparing base (55fb1b1) to head (2317a81).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1724   +/-   ##
=======================================
  Coverage   61.59%   61.59%           
=======================================
  Files          84       84           
  Lines       21493    21493           
  Branches     3176     3176           
=======================================
  Hits        13239    13239           
  Misses       6078     6078           
  Partials     2176     2176           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sbryngelson]

Reviewed and pushed two fixes to this branch (89c0f71) — thanks for tracking down the cross-backend abort, the diagnosis is right.

Confirmed the premise: GPU_DATA evaluates both backends at fypp time and only selects between them later with #if (parallel_macros.fpp:148-157), so the old #:stop in OMP_NOCREATE_STR broke OpenACC builds too, even though OpenACC supports no_create natively. The OpenACC path now expands correctly, and all three doc rows are covered.

Two things the change left open:

1. A clause-less !$omp target data (the reason for the code change). With the #:stop gone, OMP_NOCREATE_STR returns '', and in OMP_DATA the default parameter is None by default so OMP_DEFAULT_STR contributes nothing either. clause_val can end up completely empty:

#:call GPU_DATA(no_create='[a]')
    a = 1
#:endcall GPU_DATA

#elif defined(MFC_OpenMP)
!$omp target data
    a = 1
!$omp end target data

Reproduced identically on NVHPC, Cray and LLVMFlang. OpenMP requires target data to carry at least one map, use_device_ptr or use_device_addr clause, so an OpenMP build hits a Fortran compile error at a generated line — a worse message than the fypp one it replaced. It only bites when no_create is the sole clause; with any other clause the directive is valid and simply ignores no_create, which is the documented no-op and fine.

Fixed by emitting the body alone when there are no clauses, matching what GPU_DATA's own #else branch already does when neither backend is enabled. Worth noting a #:stop could not be used here — that would recreate exactly the cross-backend abort this PR fixes.

2. The justifying comment. "OMP_DEFAULT_STR already maps present-by-default" holds only for CCE, which emits defaultmap(present:allocatable/pointer), and only when the caller passes default='present'. NVHPC/PGI and the fallback emit defaultmap(tofrom:...) (copies rather than reuses), AMD emits nothing, and in OMP_DATA default is None so nothing is emitted at all. The no-op behavior is fine and now documented — the comment just shouldn't promise mapping semantics only one compiler delivers. Reworded.

Generated code is byte-identical to your commit for every existing call site (3 compilers x 7 source files, 29785 lines) — no call site in src/ passes no_create today, so nothing in the tree changes.

Not touched, for the record: the parallel #:stop 'Not Supported Yet' for default='none' at omp_macros.fpp:36 is the same bug class, but it is pre-existing and out of scope here, as your description notes.

The GPU CI failures on this PR are unrelated infrastructure — the Phoenix job was preempted (state=PREEMPTED), and the four Case Opt cancels are downstream of it. The test run that completed reported 0 failed. The new push should re-run them.