Skip to content

Fix MT random seed and SPLIT KISS random state restore - #2620

Draft
mads-bertelsen-agentic wants to merge 2 commits into
mccode-dev:mainfrom
mads-bertelsen-agentic:RNG_fixes
Draft

Fix MT random seed and SPLIT KISS random state restore#2620
mads-bertelsen-agentic wants to merge 2 commits into
mccode-dev:mainfrom
mads-bertelsen-agentic:RNG_fixes

Conversation

@mads-bertelsen-agentic

Copy link
Copy Markdown

Free-form text area

Please describe what your PR is adding in terms of features or bugfixes:

While looking around in the codebase I think I stumbled over a couple of problems with the random number generation that I here propose a fix for. This was discovered while using AI to analyze the current systems. Let me know if I misunderstood something and any of these fixes are actually necessary!

1: Mismatch in defines with MT causing seed to be ignored

mccode/src/cogen.c.in:2422-2424

#define _RNG_ALG_MT   1
#define _RNG_ALG_KISS 2

MT initialization is performed only by:
mccode-r.c:4259-4270

void mt_srandom(uint32_t seed) {
    mt[0] = seed;
    ...
}

mccode_main.c

#if RNG_ALG == 2
  mt_srandom(mcseed);
#endif

The above RNG_ALG sets the MT seed only when KISS is used instead of MT. Simple fix of RNG_ALG == 1 instead.

2: When the KISS random number generator is used (default), SPLIT needs to copy and restore the random state of each ray to continue the random number generation instead of redoing the same RNG. The KISS random number generate has 7 values on the particle struct, but only the first is copied / restored, so the quality of random numbers in SPLIT is reduced, easy fix to loop over the included values.

Questions in draft stage:

  • Does this change in cogent.c.in constitute a change in the code generator that need a design decision entry?
  • Test of MT, I did the run below, is that the correct approach?
MCSTAS_CFLAGS_OVERRIDE="-DRNG_ALG=1" \
mctest --instr=Unittest_SPLIT --testdir=run_split_mt --ncount 1E7 --limit 35 --skipnontest --mpi=3 

Declaration of use of AI-tools

  • Please add a checkmark here if you used AI-tools during the work for this contribution
  • Furter, please describe how / where and for what the tools were used:

Development OS / boundary conditions

Please describe what OS you developed and tested your additions on, and if any special dependencies are required:


PR Checklist for contributing to McStas/McXtrace

For a coherent and useful contribution to McStas/McXtrace, please fill in relevant parts of the checklist:

  • My work touches the code-generator in mccode/src

    • I have added reasoning and documentation for the change through an ADR record in our GRAMMAR section
    • I am attaching test output in the comments
  • My work touches / adds to the runtime lib code (.c,.h etc in multiple locations

    • I am have added reasoning and documentation for the change below
    • I am attaching test output in the comments
  • My PR is meant to fix a specific, existing issue

    • I have indicated the issue number here:
    • I have added documentation for the fix and possible side effects
  • My contribution contains something else

    • Explanation is added in free form text above or below the checklist

Preserve all seven KISS RNG words when generated CPU and regular GPU SPLIT loops restore the particle state. The previous scalar backup only carried randstate[0] and reset the remaining KISS state words on every repetition.

Correct the MT initialization guard in the common runtime so mt_srandom(mcseed) runs for RNG_ALG=1, matching the selector emitted by the code generator and ensuring command-line seed input is applied to MT.

Validated with the Unittest_SPLIT instrument for KISS and MT at SPLITS=1, 10, and 100, including deterministic same-seed and differing-seed checks.
Emit RANDSTATE_LEN alongside randstate_t for the selected MT or KISS algorithm before generating the particle and SPLIT code.

Generate normal SPLIT backup and restore loops from RANDSTATE_LEN instead of assuming the seven-word KISS state. Guard the runtime header definitions so the generated preamble remains authoritative.

Leave the FUNNEL implementation unchanged for a separate follow-up.
@mads-bertelsen-agentic
mads-bertelsen-agentic marked this pull request as draft September 1, 2026 10:24
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