Skip to content

Single_crystal without a reflection list, and an OpenMP target backend (-DOMPTARGET) - #3040

Draft
willend wants to merge 20 commits into
mainfrom
single-crystal-reservoir-selection
Draft

willend wants to merge 20 commits into
mainfrom
single-crystal-reservoir-selection

Conversation

@willend

@willend willend commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

This PR has two parts:

  1. Single_crystal selects the reflection during the search and keeps no list by default (numerical change, below). -DMCSX_REFL_SLIST brings the list back on CPU for SPLIT.
  2. A new OpenMP target offload backend, -DOMPTARGET, next to OpenACC. It needs no unified memory, so it suits LUMI, where HSA_XNACK is unavailable. mcstas NMX_simplified.instr now gives a .c that builds for it with no hand edits.

OMPTARGET backend

  • Macros: -DOPENACC and -DOMPTARGET are mutually exclusive. MCGPU means "device build", and OPENACC stays for the OpenACC API. The guard rename covers the runtime and the ported components only; other components keep OPENACC, which still works for OpenACC builds.
  • Code generator: omp_twin() emits the OpenMP counterpart of each #pragma acc line behind #ifdef OMPTARGET, so components need no pragma edits.
  • Transfers: a component uploads its heap data at the end of INITIALIZE and downloads at the start of SAVE, keeping device addresses in *_d members. mcgpu_* helpers in mccode-r.c check every transfer, list it on the master rank, and stop on failure.
  • Ported components, those NMX_simplified uses: Arm, DiskChopper, DivLambda_monitor, Divergence_monitor, ESS_butterfly, Elliptic_guide_gravity, Guide_m, Monitor_nD, PSD_monitor, Progress_bar, Single_crystal, Slit, TOFLambda_monitor.
  • Also ported, for wider test coverage: Guide_gravity, Guide, Guide_channeled, Source_simple, Source_div, Source_gen, Al_window, Beamstop, Collimator_linear, Monochromator_flat, Incoherent, L_monitor, E_monitor, TOF_monitor, Monitor, PSD_monitor_4PI, Brilliance_monitor. reflect in Guide and Guide_gravity and the table files in Source_gen are refused. E_monitor does not print its <E> line under OMPTARGET. Monitor now keeps its sums in an array. OFF geometry stops the build with #error under OMPTARGET.
  • Everything else is refused at compile time: a component without #define MCGPU_PORTED_<name> gives #error under OMPTARGET. FUNNEL is refused too, and so are Monitor_nD auto limits and n1D mode, Guide_m reflectivity tables and Elliptic_guide_gravity segments, at run time.

mcrun --openmp builds it, the OpenMP twin of --openacc; the two are mutually exclusive. New config entries OMP and OMPFLAGS default to cc and -O2 -fopenmp -DOMPTARGET, the LUMI build. The Cray wrapper gets the target from craype-accel-amd-gfx90a; other compilers need their own values in the user config. Older user configs keep working. mctest --openmp and the mcgui run menu follow; mctest reports any instrument with an unported component as a compile failure, which is expected for now.

OMPTARGET testing

All on macOS arm64 with clang 21. Not yet run on a real GPU.

  • No change for existing builds: old and new generators on 8 instruments, including Monitor_nD_noacc and FUNNEL. CPU and -DOPENACC preprocessed output is identical apart from the new *_d members, and #pragma acc lines are unchanged for the three instruments compared.
  • -DOMPTARGET NMX on a stand-in device: host memory acts as the device, and host arrays are overwritten after upload so that reading a host pointer shows up. At -n 1e8 -s 1234, all 67 events, six monitor totals and three monitor arrays match the reference, with real threads.
  • mctest --openmp vs CPU on a stand-in device, 57 instruments, 75 test values: 49 ran and all are identical to the CPU. The other 26 are refusals: Monitor_nD n1D mode (20), unported Monochromator_curved (4), OFF geometry (2).
  • gfx90a: device compile succeeds with clang, and the image references no libc, only the OpenMP device runtime and math functions.
  • Refusals: OPENACC + OMPTARGET, FUNNEL, and unported components all stop the build as intended.

Still to do: a LUMI run with CCE, including whether CCE defines __AMDGCN__ in its device pass; the matching generator changes in mccode-antlr.

Single_crystal: what changes

McStas Single_crystal now picks the reflection during the search and stores no reflection list, on CPU and GPU alike.

  • hkl_search_select replaces hkl_search + hkl_select. A weighted reservoir draw keeps one reflection with P = refl_j / Σrefl.
  • The T[MCSX_REFL_SLIST_SIZE] array is gone from each GPU thread's stack, about 172 kB per thread at size 1024.
  • Reachable reflections are no longer capped. On NMX with a list of 1024, the cap was exceeded in 40% of searches, losing about 23% of coh_refl in those searches.
  • Reflections sort on a 1e-9 grid in tau, then h, k, l. Rounding differences such as FMA used to reorder symmetry equivalents, so CPU and GPU picked different, mirrored reflections from the same random draw.

Removed

  • By default, the CPU SPLIT cache and its statistics, including the "use SPLIT" hint at the end of a run. Each SPLIT iteration now searches and draws its own reflection.
  • The USE_OPENCL branch. It could not link: hkl_search_opencl is not defined anywhere and nothing initialises oclContext_SX. opencl-lib and hkl_search.cl are left for separate cleanup.

Optional list on CPU: -DMCSX_REFL_SLIST

The list and the SPLIT reuse of the search are still worth having on CPU, so this define brings them back, with the cache, its statistics and warnings. Size comes from MCSX_REFL_SLIST_SIZE (default 128), and deltak stays as a parameter, unused otherwise. Both modes share one search body. GPU builds ignore the define.

The switch is a new define on purpose. MCSX_REFL_SLIST_SIZE alone has no effect, because instruments such as NMX_simplified already set it, and using it as the switch would make their CPU and GPU builds select differently.

Numerical change

Fixed-seed results change for every McStas instrument using Single_crystal, because each reachable reflection now draws a random number and the sort order changed. Statistics match the old version, except that searches which overflowed the old list now keep the reflections it dropped. This needs a line in CHANGES_McStas at release time.

Single_crystal testing

All on macOS arm64 with clang, on the CPU.

  • NMX_simplified, -n 1e8 -s 1234, generated from the .instr: all 67 detector events and all monitor totals checked match the reference run row for row. That reference is the hand-edited EPICURE version with the same algorithm, whose CPU and OpenACC runs agreed with each other.
  • Old vs new component, 11 parameter sets, 54 monitors: Test_SX (including SPLITS=10, order=1), Unittest_SPLIT_sample (SPLITS=1, 100), Test_Monochromators (Mono=6, PG=1, powder=1), templateNMX, templateLaue (2 files). Largest |z| is 1.30.
  • -DMCSX_REFL_SLIST against the old component with only the sort changed: bit-identical output at fixed seed, 15 files across Test_SX (order=1 SPLITS=10, SPLITS=5) and templateNMX.
  • Run time: Test_SX with order=1 SPLITS=10 takes 7.5 s by default and 4.1 s with -DMCSX_REFL_SLIST, as with the old component. Test_SX with SPLITS=5 and templateNMX show no difference.

Not tested:

  • This exact component on GPU. The OpenACC evidence comes from the hand-edited version, and the code here was tidied slightly since (T is a single struct, hkl_search_select returns void).
  • Linux, Windows/MSVC, or gcc.
  • The full mctest suite.

Scope

McXtrace is deliberately left out. Its Single_crystal and FluoCrystal keep the list and the old sort, no McXtrace component is changed, and McXtrace was not built or tested, so the shared generator and runtime changes could affect it.

🤖 Generated with Claude Code

hkl_search_select replaces hkl_search + hkl_select: a weighted reservoir
draw keeps one reflection with P = refl_j / sum(refl), so no list exists
on CPU or GPU. This removes the per-thread T[MCSX_REFL_SLIST_SIZE] array
and the silent cap on reachable reflections.

Also remove what depended on the list: the CPU SPLIT cache and its stats,
the deltak parameter, MCSX_REFL_SLIST_SIZE, and the USE_OPENCL branch,
which could not link (hkl_search_opencl is not defined anywhere).

Reflections now sort on a 1e-9 tau grid, then h, k, l, so the order of
symmetry equivalents no longer depends on compiler rounding (FMA).

Numerical change: fixed-seed results change for all Single_crystal
instruments. Statistics agree with the old version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willend
willend marked this pull request as draft November 4, 2026 12:53
OMPTARGET sits next to OpenACC and is mutually exclusive with it. MCGPU now
means "device build"; OPENACC stays for the OpenACC API only. It runs without
unified memory, so it works on LUMI where HSA_XNACK is unavailable.

Code generator: omp_twin() emits the OpenMP counterpart of each #pragma acc
line behind #ifdef OMPTARGET, covering generated code, runtime and components.
Also the backend macros, the device-side instrument pointer, and an #error for
any component without MCGPU_PORTED_<name>.

Runtime: checked transfer helpers (mcgpu_*), MC2D, no libc in the device pass,
MPI device selection.

Components: only the types used by NMX_simplified are ported. Each one moves
its own heap data, uploading in INITIALIZE and downloading in SAVE, and keeps
the device address in *_d members. FUNNEL is refused.

No numerical change: CPU and OPENACC builds preprocess to the same code as
before, apart from the new *_d struct members. McXtrace components are not
changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@willend willend changed the title Single_crystal: select the reflection during the search, keep no list Single_crystal without a reflection list, and an OpenMP target backend (-DOMPTARGET) Nov 4, 2026
willend and others added 18 commits November 4, 2026 14:24
The list and the SPLIT reuse of the search are a real gain on CPU: 1.8x with
order=1 SPLITS=10 on Test_SX. -DMCSX_REFL_SLIST brings them back, sized by
MCSX_REFL_SLIST_SIZE (default 128), and restores the deltak parameter. Both
modes share one search body. GPU builds ignore the option.

The switch is a new define because instruments such as NMX_simplified set
MCSX_REFL_SLIST_SIZE already, and would otherwise differ between CPU and GPU.

Numerical check: with the same sort, list mode is bit-identical to the old
component at fixed seed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The message claimed OpenACC for OMPTARGET builds too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mcgpu_die uses MPI_MASTER, which needs mpi_node_rank, so an OMPTARGET
build with -DUSE_MPI did not compile.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
New OMP and OMPFLAGS entries in the configuration, default cc and
"-O2 -fopenmp -DOMPTARGET", which is the LUMI build (the Cray wrapper
takes the target from craype-accel-amd-gfx90a). Other compilers set
them in the user configuration. --openacc and --openmp are mutually
exclusive.

User configurations from before this change keep working: the loaded
compilation section now updates the defaults instead of replacing them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mctest --openmp passes the flag to mcrun, like --openacc, and the two
are mutually exclusive. mcgui gets the four OpenMP entries next to the
OpenACC ones in the parallelisation menu.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The flags unsupported by nvc were only removed when the OMP compiler
name contained nvc, so the default cc on LUMI, or amdclang, got them
from a conda environment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The component data is all inside its struct, so nothing needs moving.
A reflect table stays host heap and is refused, as in Guide_m, and the
warning fprintf in TRACE is left out of the device build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Source_simple, Source_div, Source_gen, L_monitor, E_monitor and
TOF_monitor. The monitors follow PSD_monitor, using a new MC1D macro.

Source_gen refuses its table files, as Guide_m does. E_monitor leaves
its S_p sums out of the device build, so the <E> line is not printed.

No numerical change: CPU output is identical before and after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A long --instr or --comp list went into the label unchanged, so mkdir
failed with "File name too long". Keep the first 100 characters and warn.
An explicit --suffix still replaces a long filter name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Instruments can set -DUSE_OFF themselves, and off_struct data is host
heap, so Monitor_nD and Single_crystal would have read host pointers on
the device. Stop the build with a clear #error instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Al_window, Beamstop, Collimator_linear, Guide_channeled,
Monochromator_flat and Incoherent need only the marker. Guide refuses
reflect tables, as Guide_m does. PSD_monitor_4PI and Brilliance_monitor
follow PSD_monitor.

Monitor keeps its three sums in an array instead of scalars, so that
they can come back from the device, and gains a FINALLY section.

No numerical change: CPU output is identical before and after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pnorm2 was Pnorm + Pnorm, so the errors of the TOF-wavelength map and
the peak brilliance were wrong. Intensities are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A neutron at theta == PI or phi == PI/2 gave index nx or ny, writing
past the array. Such events now go in the last bin. y / radius above 1
by rounding gave a NaN index, and is now limited to 1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Same fix as the McStas component: a ray at theta == PI or psi == PI/2
gave index nx or ny, writing past the array, and y / radius above 1 by
rounding gave a NaN index.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Monitor_nD_EnsureCapacity from main is host only, so its call in
Monitor_nD is now guarded by MCGPU instead of OPENACC.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant