Skip to content

Remove need for Union_init and Union_stop - #2655

Draft
mads-bertelsen-agentic wants to merge 15 commits into
mccode-dev:mainfrom
mads-bertelsen-agentic:Avoid_union_init_and_stop
Draft

Remove need for Union_init and Union_stop#2655
mads-bertelsen-agentic wants to merge 15 commits into
mccode-dev:mainfrom
mads-bertelsen-agentic:Avoid_union_init_and_stop

Conversation

@mads-bertelsen-agentic

Copy link
Copy Markdown
Contributor

Free-form text area

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

This is a suggestion on how to avoid the need for the Union_init and Union_stop components. Today they serve
two functions:

  • Add the .c code files at start and end so the preprocessor can limit what code is actually compiled
  • Provide "global" variables in the init, other Union components communicated through GETPAR things

This branch simply does these things in the cogen by checking if a Union_master or Union_master_GPU is in the instrument. This is to avoid more invasive things like tagging all the Union components, but other suggestions for detecting when to trigger including union globals and libraries.

Instruments with Union_init and Union_stop will continue to work, just show deprecation warnings. Instruments that use the init parameter on Union components explicitly will break as that parameter is not available anymore (mcstas-pygen generated instruments might be extra susceptible to this, as all parameters are always specified).


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 contribution includes patches to an existing component file

    • I have used the mcdoc utility and rendered a reasonable documentation page for the component (please attach as screenshot in comments!)
    • I have ensured that basic use of the component is OK (e.g. an instrument using it compiles?)
    • I have used the mctest utility to test one or more instruments making use of the component (please attach mcviewtest report as screenshot in comments)
    • I have used the mccode-clangformat tool to apply the standard McCode component indentation scheme
    • I have used the mcrun --c-lint "linter" and followed advice to remove most / all warnings that are raised
  • My contribution includes patches to an existing instrument file

    • I have used the mcdoc utility and rendered a reasonable documentation page for the instrument (please attach as screenshot in comments!)
    • I have used the mctest utility to test the instrument (please attach mcviewtest report as screenshot in comments)
    • I have used the mcrun --c-lint "linter" and followed advice to remove most / all warnings that are raised
  • My contribution includes a new component file

    • I have ensured that naming of parameters are in the style of existing components. (Please check the McStas or McXtrace NOMENCLATURE docs.)
    • I have ensured that component parameters are in the usually units of McStas or McXtrace (SI + neutron/x-ray 'usual' units)
    • I have used the mcdoc utility and rendered a reasonable documentation page for the component (please attach as screenshot in comments!)
    • I have ensured that basic use of the component is OK (e.g. an instrument using it compiles?)
    • I have included a corresponding example instrument and will fill in the new instrument section below
    • I have used the mccode-clangformat tool to apply the standard McCode component indentation scheme
    • My new component is added within the contrib component category
  • My contribution includes a new instrument file

    • I have used the mcdoc utility and rendered a reasonable documentation page for the instrument (please attach as screenshot in comments!)
    • I have ensured that basic use of the instrument is OK (e.g. it compiles?)
    • ... and provided reasonable default parameters in that instrument that produce reasonable output
    • ... and maybe even added a %Example: line to describe expected behaviour
    • I have used the mcrun --c-lint "linter" and followed advice to remove most / all warnings that are raised
    • My new instrument is added within the examples hierarchy in a folder in the style of examples/ESS/New_stuff/New_stuff.instr
    • My new instrument has a new, unique filename, not clashing with existing example instruments
    • My new instrument requires a data/input file. If the datafile is specific for my instrument I have left it in the same example folder, but if general use I have placed it in the global data folder.
  • 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

mads-bertelsen-agentic and others added 15 commits September 9, 2026 06:40
Union instruments previously required a Union_init component placed first
and a Union_stop component placed last. Union_init inlined the shared
union-lib.c header (types) and union-init.c (the global list state,
accessed through COMP_GETPAR3(Union_init, init, ...)); Union_stop inlined
union-suffix.c (the dispatch switch) exactly once, after every
PROCESS_*_DETECTOR macro had already been defined. This made the required
first/last ordering fragile.

cogen now injects that shared front/back automatically, so Union_init and
Union_stop are no longer required. The change is applied to both mcstas and
mcxtrace in lockstep.

Detection
cogen_instr_uses_union() returns true when the instrument contains a
Union_master or Union_master_GPU component (the master is mandatory when any
Union component is used). The shared code is injected only in that case, so
non-union instruments are byte-for-byte unchanged.

Injection (mccode/src/cogen.c.in)
- Front, at the top of cogen_decls (before the SHARE loop): emit
  "#define Union 1", then embed_file("union-lib.c") and
  embed_file("union-init.c").
- Back, after the SHARE loop: embed_file("union-suffix.c").
- embed_file() is include-once (via the symtab) and searches the system
  directory before the component path, so each shared file is emitted
  exactly once even when many union components are present.

Shared state (union-init.c, both compilers)
The file-scope globals are renamed to a g_ prefix so they cannot collide
with the component-local pointers of the same name, e.g.
global_process_list -> g_process_list. Each component keeps its local
pointer but sets it to &g_FOO, so all ->, . and * usages (and the
..._master LHS names) are left untouched.
- mcstas: 13 globals (incl. g_surface_list, g_mantid_min_pixel_id)
- mcxtrace: 11 globals (no surface / mantid_min_pixel_id)

Components (mcstas-comps/union, mcxtrace-comps/union)
- Every union .comp now uses &g_FOO instead of
  COMP_GETPAR3(Union_init, init, global_FOO); the
  "if (_getcomp_index(init) < 0) {...}" guard blocks are removed.
- mcxtrace components no longer %include union-init.c (cogen injects it);
  Template_process.comp's bare &global_process_list becomes &g_process_list.
- Int special case: Union_abs_logger_1D_space_event.comp now reads
  g_mantid_min_pixel_id directly (no &, cast dropped).
- Function parameters named global_* in SHARE signatures are untouched.
- Union_init.comp / Union_stop.comp are converted to inert no-ops (empty
  SHARE/DECLARE/INITIALIZE), so existing .instr files that still place them
  compile and run unchanged. The deprecated string init="init" parameter is
  kept so old instruments still parse.

Include guards
union-lib.c, union-init.c and union-suffix.c each gained a guard
(UNION_LIB_C / UNION_INIT_C / UNION_SUFFIX_C) in both mcstas and mcxtrace.

Verification
- mcstas mctest passes: old-style instruments (with init/stop) unchanged;
  new-style (no init/stop) reproduce the exact detector value (4.55997e-10);
  non-union instruments show no injection markers; the dispatch switch is
  emitted once with the PROCESS_*_DETECTOR macros defined before it.
- mcxtrace: Geometry_test compiles with the injection and runs to completion
  (Union_master initialized, trace saved; no .nxs counts because all
  DETECTORs are commented out in that example). Its meshfile mcrun prompt is
  pre-existing (the example requires an external torus.STL it does not ship)
  and reproduces identically on the unmodified baseline.
Every Union component (except the no-op Union_init/Union_stop) now
emits a clear #error when the shared Union library was not injected,
which happens when the instrument contains no Union_master /
Union_master_GPU. Previously mcstas carried a stale message pointing at
the now-optional Union_init component, and mcxtrace silently defined
Union so a masterless instrument failed only with cryptic
'incomplete type' errors.

Also drop the dead, never-referenced length_to_boundery_fp
declaration in Union_master (mcxtrace, missing semicolon) and
Union_master_GPU (mcstas).
@mads-bertelsen-agentic
mads-bertelsen-agentic marked this pull request as draft September 11, 2026 15:21
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.

2 participants