Repository navigation
Conversation
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
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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has two parts:
Single_crystalselects the reflection during the search and keeps no list by default (numerical change, below).-DMCSX_REFL_SLISTbrings the list back on CPU for SPLIT.-DOMPTARGET, next to OpenACC. It needs no unified memory, so it suits LUMI, whereHSA_XNACKis unavailable.mcstas NMX_simplified.instrnow gives a.cthat builds for it with no hand edits.OMPTARGET backend
-DOPENACCand-DOMPTARGETare mutually exclusive.MCGPUmeans "device build", andOPENACCstays for the OpenACC API. The guard rename covers the runtime and the ported components only; other components keepOPENACC, which still works for OpenACC builds.omp_twin()emits the OpenMP counterpart of each#pragma accline behind#ifdef OMPTARGET, so components need no pragma edits.*_dmembers.mcgpu_*helpers inmccode-r.ccheck every transfer, list it on the master rank, and stop on failure.Arm,DiskChopper,DivLambda_monitor,Divergence_monitor,ESS_butterfly,Elliptic_guide_gravity,Guide_m,Monitor_nD,PSD_monitor,Progress_bar,Single_crystal,Slit,TOFLambda_monitor.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.reflectinGuideandGuide_gravityand the table files inSource_genare refused.E_monitordoes not print its<E>line under OMPTARGET.Monitornow keeps its sums in an array. OFF geometry stops the build with#errorunder OMPTARGET.#define MCGPU_PORTED_<name>gives#errorunderOMPTARGET.FUNNELis refused too, and so are Monitor_nD auto limits and n1D mode,Guide_mreflectivity tables andElliptic_guide_gravitysegments, at run time.mcrun --openmpbuilds it, the OpenMP twin of--openacc; the two are mutually exclusive. New config entriesOMPandOMPFLAGSdefault toccand-O2 -fopenmp -DOMPTARGET, the LUMI build. The Cray wrapper gets the target fromcraype-accel-amd-gfx90a; other compilers need their own values in the user config. Older user configs keep working.mctest --openmpand themcguirun 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.
Monitor_nD_noaccandFUNNEL. CPU and-DOPENACCpreprocessed output is identical apart from the new*_dmembers, and#pragma acclines are unchanged for the three instruments compared.-DOMPTARGETNMX 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 --openmpvs 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_nDn1Dmode (20), unportedMonochromator_curved(4), OFF geometry (2).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 inmccode-antlr.Single_crystal: what changes
McStas
Single_crystalnow picks the reflection during the search and stores no reflection list, on CPU and GPU alike.hkl_search_selectreplaceshkl_search+hkl_select. A weighted reservoir draw keeps one reflection with P = refl_j / Σrefl.T[MCSX_REFL_SLIST_SIZE]array is gone from each GPU thread's stack, about 172 kB per thread at size 1024.coh_reflin those searches.Removed
USE_OPENCLbranch. It could not link:hkl_search_openclis not defined anywhere and nothing initialisesoclContext_SX.opencl-libandhkl_search.clare left for separate cleanup.Optional list on CPU:
-DMCSX_REFL_SLISTThe 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), anddeltakstays 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_SIZEalone 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 inCHANGES_McStasat release time.Single_crystal testing
All on macOS arm64 with clang, on the CPU.
-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.Test_SX(includingSPLITS=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_SLISTagainst the old component with only the sort changed: bit-identical output at fixed seed, 15 files acrossTest_SX(order=1 SPLITS=10,SPLITS=5) andtemplateNMX.Test_SXwithorder=1 SPLITS=10takes 7.5 s by default and 4.1 s with-DMCSX_REFL_SLIST, as with the old component.Test_SXwithSPLITS=5andtemplateNMXshow no difference.Not tested:
Tis a single struct,hkl_search_selectreturns void).Scope
McXtrace is deliberately left out. Its
Single_crystalandFluoCrystalkeep 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