Skip to content

Add MCViNE sample kernels as Union processes (and standalone samples) - #2716

Open
Fahima-Islam wants to merge 5 commits into
mccode-dev:mainfrom
Fahima-Islam:mcvine-kernels
Open

Fahima-Islam wants to merge 5 commits into
mccode-dev:mainfrom
Fahima-Islam:mcvine-kernels

Conversation

@Fahima-Islam

Copy link
Copy Markdown
Contributor

Free-form text area

This PR adds 16 sample scattering kernels from MCViNE (https://github.com/mcvine/mcvine, mccomponents/lib/kernels/sample, plus SANS2D\_ongrid from https://github.com/mcvine/acc). McStas has no equivalent for any of them. Each kernel is available in two forms:

  • a Union process contrib/MCViNE\_<kernel>\_process.comp (geometry, containers, absorption and multiple scattering by Union), and
  • a standalone sample contrib/MCViNE\_<kernel>.comp (box / (hollow) cylinder / sphere, same transport as MCViNE's HomogeneousNeutronScatterer).

Both forms call the same kernel code in share/mcvine-lib.c.

Kernels added

Kernel Physics
ConstantEnergyTransfer, ConstantQE, ConstantvQE fixed ΔE / fixed (|Q|,E) / fixed Q vector (resolution and test kernels)
E_Q, Broadened_E_Q, LorentzianBroadened_E_Q isotropic dispersion E(|Q|) with S(Q), optionally Gaussian or Lorentzian broadened
E_vQ single-crystal dispersion E(Qx,Qy,Qz) with S(Qx,Qy,Qz)
SQ, SvQ elastic S(|Q|) or S(Q) (expression or grid)
SQE isotropic S(Q,E) from an expression or an Isotropic\_Sqw-format grid, optional final-energy focusing
DGSSXRes direct-geometry single-crystal resolution kernel
Phonon_IncoherentElastic, Phonon_IncoherentInelastic incoherent elastic with Debye–Waller factor; one-phonon incoherent scattering from a DOS (optional Ef focusing)
Phonon_CoherentInelastic_PolyXtal, Phonon_CoherentInelastic_SingleXtal coherent one-phonon scattering from a phonon dispersion + polarization vectors on a grid (MCViNE IDF files, e.g. from phonopy)
SANS2D_ongrid SANS from a 2D S(Qx,Qy) map

MCViNE takes functions such as E(Q) and S(Q) as strings evaluated at run time (fparser). The same is supported here through a small built-in expression evaluator, e.g. E\_Q="20\*sin(Q\*1.5)^2".

Kernels not added, because McStas already covers them: isotropic scattering (Incoherent), gridded S(Q,E) (Isotropic\_Sqw), powder and single-crystal diffraction (PowderN, Single\_crystal), SANS spheres, and multiphonon (IncoherentPhonon\_process, NCrystal). MCViNE's EPSC diffraction kernel is unfinished upstream and is not ported.

Runtime-library change (Union core, 12 added lines). Union dispatches physics with a switch on enum process. This PR registers one new process type, MCViNE:

  • share/union-lib.c: adds an enum entry and a data\_transfer\_union member
  • share/union-suffix.c: adds two case MCViNE: blocks guarded by PROCESS\_MCVINE\_DETECTOR, the same pattern as the other processes

All 16 kernels share this type. Its storage struct holds pointers to the kernel and to its sampling function, so further MCViNE kernels need no more core changes. Existing processes are untouched. The processes are NOACC (CPU only).

New files

  • share/mcvine-lib.h/.c – kernel physics, expression evaluator, table/grid readers (files found through Open\_File), DOS and Debye–Waller helpers, MCViNE IDF phonon readers, shapes and standalone transport
  • share/mcvine-union-lib.h/.c – Union glue
  • contrib/MCViNE\_\*\_process.comp (16), contrib/MCViNE\_\*.comp (16)
  • contrib/doc/MCViNE\_kernels.md – overview, input formats, differences from MCViNE, validation
  • examples/Tests\_samples/Test\_MCViNE\_\*/ (16 test instruments)
  • data/MCViNE/ – small test inputs (toy fcc phonon IDF set and atoms file, Debye DOS, S(q,w) grid, SANS map; ~0.6 MB)

Where the port differs from MCViNE (documented in contrib/doc/MCViNE\_kernels.md):

  • Absorbed events: kinematically forbidden events are absorbed; MCViNE passes them on unchanged.
  • Grids: grid data is interpolated rather than looked up bin by bin.
  • Two bug fixes: mcvine/acc's SANS2D velocity update, and the PolyXtal "two E_f choices" factor.
  • unbiased=1 option: available on four kernels whose MCViNE sampling loops are slightly biased; the default reproduces MCViNE.

Tests

  • Test instruments: each Test\_MCViNE\_<kernel> instrument runs the same sample as the Union process (comp\_select=1) or as the standalone component (comp\_select=2), with the same geometry, absorption and multiple scattering. It has an %Example line for each form.
  • mctest results: all 32 %Example cases pass in mctest --no-mpi at 100 % of their recorded values. The Union and standalone values of each kernel agree within about 2σ (0.3–1 % for most kernels; SQE, SANS2D and SingleXtal have larger statistical errors).
Test Union Standalone ratio
Test_MCViNE_Broadened_E_Q 5.698e-07 5.68e-07 1.003 ± 0.004
Test_MCViNE_ConstantEnergyTransfer 6.369e-07 6.362e-07 1.001 ± 0.003
Test_MCViNE_ConstantQE 6.362e-07 6.383e-07 0.997 ± 0.003
Test_MCViNE_ConstantvQE 3.172e-07 3.157e-07 1.005 ± 0.003
Test_MCViNE_DGSSXRes 6.476e-13 6.503e-13 0.996 ± 0.003
Test_MCViNE_E_Q 5.519e-07 5.529e-07 0.998 ± 0.004
Test_MCViNE_E_vQ 1.77e-07 1.788e-07 0.990 ± 0.008
Test_MCViNE_LorentzianBroadened_E_Q 5.71e-07 5.691e-07 1.003 ± 0.004
Test_MCViNE_Phonon_CoherentInelastic_PolyXtal 1.273e-07 1.266e-07 1.005 ± 0.017
Test_MCViNE_Phonon_CoherentInelastic_SingleXtal 6.618e-08 6.531e-08 1.013 ± 0.020
Test_MCViNE_Phonon_IncoherentElastic 9.022e-07 9.005e-07 1.002 ± 0.003
Test_MCViNE_Phonon_IncoherentInelastic 1.457e-07 1.452e-07 1.003 ± 0.004
Test_MCViNE_SANS2D_ongrid 9.722e-07 9.425e-07 1.032 ± 0.021
Test_MCViNE_SQE 2.433e-06 2.247e-06 1.083 ± 0.049
Test_MCViNE_SQ_elastic 6.366e-07 6.38e-07 0.998 ± 0.004
Test_MCViNE_SvQ 3.443e-07 3.433e-07 1.003 ± 0.003
  • Physics validation (scripts attached): analytic integrals, McStas Incoherent, and independent Python calculations. Examples:

    • S(Q)=Q² gives 57.89 ± 0.17 vs 2k² = 57.91.
    • The Debye–Waller core from the DOS is 0.004309 Ų, vs 0.004316 Ų by independent quadrature.
    • One-phonon incoherent intensity is 0.1543 vs 0.1544 by quadrature.
    • Single-crystal coherent phonons give 0.214 vs 0.207 from an independent calculation with exact eigenvectors.
    • Transport matches Incoherent within 0.3 % for single scattering, and an analog MC within 0.2 % for double scattering.
  • Documentation: mcdoc renders the parameter tables of all 32 components.

  • Formatting: mccode-clangformat has been applied.

  • Linting: the shared library is clean under gcc -std=c99 -Wall -Wextra and cppcheck --enable=warning,portability,performance.

---

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:

    Claude (Anthropic, Claude Code) was used. It compared the MCViNE and McStas code bases to find the missing kernels, ported the MCViNE C++ kernels to C (share/mcvine-lib.c), and wrote the Union glue and the 12-line Union core registration.
    I reviewed the ported physics against the MCViNE sources and the validation results.

---

Development OS / boundary conditions

Developed and tested on Linux (Ubuntu 24.04, gcc 13, x86_64) with McStas built from main @ d3ab8cf, using mcrun/mctest from the same build without MPI. No extra dependencies are needed. Two test instruments are slow at the default 1e6 neutrons in Union mode: Test\_MCViNE\_Phonon\_CoherentInelastic\_SingleXtal (about 150 s) and Test\_MCViNE\_E\_vQ (about 90 s).

---

PR Checklist for contributing to McStas/McXtrace

  • My contribution includes a new component file

    • I have ensured that naming of parameters are in the style of existing components (NOMENCLATURE: geometry, sigma\_\*, Vc, p\_transmit, target\_index/focus\_r, Union packing\_factor/interact\_fraction)
    • I have ensured that component parameters are in the usual units of McStas (m, meV, AA^-1, barn, AA^3, K)
    • I have used the mcdoc utility and rendered a reasonable documentation page for the component (screenshots attached)
    • I have ensured that basic use of the component is OK (all 32 compile and run in the test instruments)
    • 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
    • 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 (two per instrument)
    • I have used the mcrun --c-lint "linter" and followed advice to remove most / all warnings that are raised (the new library code was linted with cppcheck and is clean)
    • My new instrument is added within the examples hierarchy (examples/Tests\_samples/Test\_MCViNE\_\*/)
    • My new instrument has a new, unique filename, not clashing with existing example instruments
    • My new instrument requires a data/input file. The shared test inputs are in the global data/MCViNE/ folder.
  • My work touches / adds to the runtime lib code (.c,.h etc in multiple locations

    • I have added reasoning and documentation for the change below: see "Runtime-library change" above; the Union core only gains the MCViNE enum entry, union member and two dispatch cases
    • I am attaching test output in the comments

🤖 Generated with Claude Code

Fahima-Islam and others added 4 commits September 30, 2026 04:33
Add the MCViNE entry to enum process, a pointer_to_a_MCViNE_physics_storage_struct
member to union data_transfer_union, and the two dispatch cases in physics_my and
physics_scattering (guarded by PROCESS_MCVINE_DETECTOR). All MCViNE kernel
processes share this one type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YUUwywmxwaAvj7JoVQJ9y2
mcvine-lib: ports of the MCViNE sample kernels (mccomponents/lib/kernels/sample
and mcvine/acc SANS2D_ongrid), a run-time expression evaluator, table and grid
readers, DOS / Debye-Waller helpers, MCViNE IDF phonon readers, sample shapes and
the MCViNE HomogeneousNeutronScatterer transport.
mcvine-union-lib: glue between the kernels and the Union framework.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YUUwywmxwaAvj7JoVQJ9y2
MCViNE_<kernel>_process (Union) and MCViNE_<kernel> (standalone) for
ConstantEnergyTransfer, ConstantQE, ConstantvQE, E_Q, Broadened_E_Q,
LorentzianBroadened_E_Q, E_vQ, SQ, SvQ, SQE, DGSSXRes, SANS2D_ongrid,
Phonon_IncoherentElastic, Phonon_IncoherentInelastic,
Phonon_CoherentInelastic_PolyXtal and Phonon_CoherentInelastic_SingleXtal.
Documentation in contrib/doc/MCViNE_kernels.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YUUwywmxwaAvj7JoVQJ9y2
One Tests_samples/Test_MCViNE_<kernel> instrument per kernel, running the Union
process (comp_select=1) or the standalone component (comp_select=2) on the same
sample, with an %Example line for each. Test inputs in data/MCViNE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YUUwywmxwaAvj7JoVQJ9y2
@Fahima-Islam

Copy link
Copy Markdown
Contributor Author
examples_overview mcdoc_MCViNE_E_Q mcdoc_MCViNE_Phonon_CoherentInelastic_SingleXtal_process mcdoc_Test_MCViNE_E_Q [mctest_output.txt](https://github.com/user-attachments/files/32847866/mctest_output.txt) mcviewtest_report union_examples_overview

@willend

willend commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

@Fahima-Islam this looks absolutely great, good job! :-)
I will thus archive #2686

The compilation issue for mcstas-antlr is intermittent and will be fixed via an incoming PR.

@mads-bertelsen

Copy link
Copy Markdown
Contributor

Wow, great job! Really like how the code is organized such that duplication is avoided across the stand alone samples and Union processes. This offers a lot of exciting new capabilities! The run times are also faster than I feared for the advanced inelastic capabilities.

You mention that duplicated efforts such as 2D grid (Q,E) are not ported, and of course its extra work to do that, but in general we welcome having several independent ways to calculate the same thing for a good cross check.

@willend

willend commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

@Fahima-Islam absolutely nice stuff indeed. :-)

Just a word of warning - the Windows platform will require a bit more work it seems, but Claude might help you with that. (Pick the right artefact from https://github.com/mccode-dev/McCode/actions/runs/36691645550?pr=2716 and investigate the failing new MCVINE instrument compile_stdout.txt.)

Most often the issues are along the lines of

  • gnu style double blah[var]; allocations need replacement by malloc()'s
  • certain POSIX assumptions may need local plugs / workarounds
  • if anything is typed complex double you need to rephrase vars / expressions to use mccode-complex-lib

Screenshot of Windows state:
Screenshot 2026-09-30 at 13 17 15

@willend

willend commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

@Fahima-Islam I have dug out the Windows results for you
MCViNE_failures.tgz

It looks like a number of them indeed are things like replacements of double e1[3], e2[3], e3[3], d[3]; -> malloc's.

I will boot up my Windows later to see if I can get a hold of some of this...

rpcndr.h (pulled in by windows.h) has '#define small char', so
'int i, small;' in mcvine_S_Phonon_IncoherentInelastic became
'int i, char;' under MSVC and broke the build of every instrument
that includes mcvine-lib.c.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Fahima-Islam

Copy link
Copy Markdown
Contributor Author

@willend thanks a lot for digging out the Windows results!

All 16 Test_MCViNE_* failures turned out to have a single cause: a local variable named small in mcvine_S_Phonon_IncoherentInelastic (share/mcvine-lib.c). rpcndr.h, pulled in by windows.h, has #define small char, so under MSVC int i, small; became int i, char;. That gives exactly the C2059 / C2513 / C2181 errors in the logs, and since mcvine-lib.c goes into every MCViNE instrument, all of them failed.

5f6df3c renames it to is_small (3 lines). I checked it with the Test_MCViNE_E_Q.c generated on the Windows runner, compiled with -Dsmall=char: the original fails at the same three lines, the fixed one compiles. I haven't found other identifiers in the PR that clash with Windows macros. The fixed-size arrays (double e1[3] etc.) are fine for MSVC. The min/max C4005 warnings come from the monitor's parameters in the test instrument, not from the MCViNE code.

Let's see what the Windows CI says now.

@willend

willend commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

@Fahima-Islam great!

@willend

willend commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Seems to do he trick indeed @Fahima-Islam - snipped from the Windows compile log:

Test_MCViNE_Broadened_E_Q                       :   2.83
Test_MCViNE_ConstantEnergyTransfer              :   2.58
Test_MCViNE_ConstantQE                          :   2.58
Test_MCViNE_ConstantvQE                         :   2.69
Test_MCViNE_DGSSXRes                            :   2.62
Test_MCViNE_E_Q                                 :   2.70
Test_MCViNE_E_vQ                                :   2.71
Test_MCViNE_LorentzianBroadened_E_Q             :   2.51
Test_MCViNE_Phonon_CoherentInelastic_PolyXtal   :   2.61
Test_MCViNE_Phonon_CoherentInelastic_SingleXtal :   2.64
Test_MCViNE_Phonon_IncoherentElastic            :   2.62
Test_MCViNE_Phonon_IncoherentInelastic          :   2.64
Test_MCViNE_SANS2D_ongrid                       :   2.64
Test_MCViNE_SQE                                 :   2.67
Test_MCViNE_SQ_elastic                          :   2.51
Test_MCViNE_SvQ                                 :   2.69

FYI we are also in the midst of restructuring parts of Union, among other things leading to a solution where Union_init and Union_stop are dropped / removed. We may decide finalise that process between @mads-bertelsen, @g5t and I before we merge this PR. My hopes are still that we will make that and v3.8.9 in maximum a weeks time. :-)

@Fahima-Islam

Copy link
Copy Markdown
Contributor Author

@willend thanks for confirming! All CI has now finished:

  • Windows (McStas and McXtrace) passes; all 16 MCViNE instruments compile and run.
  • All 32 MCViNE tests (Union process and standalone for each kernel) pass on every platform, with values at 97–100 % of the %Example references.
  • ubuntu-latest.conda.antlr fails with the same single compile error as main, which you mentioned is being fixed separately.
  • ubuntu-24.04.source.classic.gcc-13 (mcstas-basictest) fails with runtime errors in ILL_H5 and ILL_H5_new. They don't use MCViNE or Union, and both passed in this PR's first CI round. Between the two rounds main gained PowderN, Single_crystal: Support multiphase and single-crystal NCrystal materials #2715 (PowderN / NCrystal multiphase) and NCrystal: Search for data files in the directory of the instrument file (localdata/) #2714 (Open_File search paths), and ILL_H5 uses PowderN and reads several data files. Since main only tests changed instruments, this may be a regression on main that only shows up in PRs that trigger the full test set. Could you take a look?

On the Union restructuring: no problem with waiting. Once Union_init / Union_stop are gone I'm happy to adapt the _process components and the test instruments to the new setup, so just ping me when it has landed.

@Fahima-Islam

Copy link
Copy Markdown
Contributor Author

@mads-bertelsen thank you! I'm glad the code structure and the run times work for you.

Good point about independent implementations as cross-checks. I left those kernels out to keep this PR focused, but they would be straightforward to add in a separate follow-up PR using the same mcvine-lib / MCViNE process-type setup, so no further Union core changes would be needed. Candidates are MCViNE's isotropic, gridded S(Q,E), powder and single-crystal diffraction, SANS sphere and multiphonon kernels, each with test instruments comparing against the existing McStas components (Incoherent, Isotropic_Sqw, PowderN, Single_crystal, the SANS samples and IncoherentPhonon_process). MCViNE's EPSC diffraction kernel is unfinished upstream, so I'd leave it out.

This kind of work has also become much faster with AI-assisted ("vibe") coding: this PR, porting 16 kernels from MCViNE's C++, the Union glue and the test instruments, came together far quicker than it would have by hand. Most of my time went into checking the physics and validating against MCViNE and the references. So a follow-up with the overlapping kernels is a realistic next step rather than a long project, and having two independent implementations side by side is exactly the kind of cross-check that makes the fast route trustworthy.

I'd start that after this PR is merged and the Union restructuring has settled, so it is built on the new Union interface. Does that sound good?

@willend

willend commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

@Fahima-Islam I've recently seen intermittent failures on
ubuntu-24.04.source.classic.gcc-13 (mcstas-basictest) in ILL_H5 and ILL_H5_new (I believe on a scale of ~ a few weeks) so I am tempted to conclude these are unrelated to both of your changes (of course :-) ) and all of the other stuff that just came in... :-)

As a side-note both of ILL_H5 and ILL_H5_new are "full guide-hall simulations" that includes a plethora of JUMPs in combination with EXTEND, WHEN, GROUP, COPY SPLIT etc... So in this way an anomaly compared to the overall examples/.

I think I will simply add an issue for that to later perform a separate investigation.

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.

3 participants