Skip to content

First attempt at making PowderN load NCMAT data directly via NCrystal#2371

Open
tkittel wants to merge 8 commits intomccode-dev:mainfrom
tkittel:ncrystal_powdern
Open

First attempt at making PowderN load NCMAT data directly via NCrystal#2371
tkittel wants to merge 8 commits intomccode-dev:mainfrom
tkittel:ncrystal_powdern

Conversation

@tkittel
Copy link
Copy Markdown
Contributor

@tkittel tkittel commented Mar 25, 2026

Free-form text area

In consultation with @willend, this PR adds the capability for the PowderN component to accept NCrystal cfg-strings directly in the reflections parameter. When this is the case, NCrystal will be used during INITIALIZE to load the reflection list (as well as V_0, sigma_a, sigma_i, at_nb, ...).

So for instance, instead of reflections="Al.laz", one can now write reflections="Al_sg225.ncmat", or reflections="Al_sg225.ncmat;temp=200K;dcutoff=0.5" or whatever else is supported in NCrystal cfg-strings. Note that the logic for detecting the reflections string as associated with NCrystal is that any string containing either the substring ".ncmat" or the substring "::" is an NCrystal cfg-string. We might want to fine-tune this to e.g. allow a .laz file to be loaded with NCrystal for debugging. Although one could write "relpath::Al.laz" in this scheme to get that effect, it might be a bit too obscure.

The idea is of course is that this makes it possible to leverage NCrystal as a source of materials also in cases where unique features of PowderN are needed (e.g. line broadening or GPU), and where features like the inelastic components of NCrystal are less important.

Other changes to the component:

  • I removed the double Epsilon field from the line_data struct, as it did not seem to do anything.
  • During testing of this PR, it came to my attention that the ncrystal_ncmat2hkl encoded values for sigma_i and sigma_a in barns/unitcell while PowderN interprets them as barns/atom. Ultimately this gives an error of a factor of natoms/unitcell. This will be fixed on the NCrystal side (see Wrong sigma_i/sigma_a in laz files from ncrystal_ncmat2hkl mctools/ncrystal#337), but the present PR also tries to make PowderN read such files correctly (irrespectively of whether the files were produced with the old or the future ncrystal_ncmat2hkl version).

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

@tkittel
Copy link
Copy Markdown
Contributor Author

tkittel commented Apr 15, 2026

mcdoc (only updated the description of the reflections parameter)

image

@tkittel tkittel marked this pull request as ready for review April 15, 2026 14:41
@willend
Copy link
Copy Markdown
Contributor

willend commented Apr 15, 2026

The “failed” test in mcstas-conda-basictest seems to be related to Test_Powders, even though nothing is evident from the table:

…
Test_Powders                :   3.34    [val: 367574000.0 / 367000000.0 = 100 %]
Test_Powders_2              :   3.17    [val: 768832.0 / 735000.0 = 105 %]
Test_Powders_3              :   3.38    [val: 390967000.0 / 420000000.0 = 93 %]
Test_Powders_4              :   3.43    [val: 781106.0 / 830000.0 = 94 %]
Test_Powders_5              :   3.85    [val: 391643000.0 / 420000000.0 = 93 %]
Test_Powders_6              :   3.90    [val: 679788.0 / 830000.0 = 82 %]
Test_Powders_7              :   3.32    [val: 467619000.0 / 420000000.0 = 111 %]
Test_Powders_8              :   3.32    [val: 820151.0 / 830000.0 = 99 %]
Test_Powders_9              :   2.05    [val: 91329000.0 / 91000000.0 = 100 %]
Test_Powders_10             :   2.11    [val: 195026.0 / 194000.0 = 101 %]
Test_Powders_11             :   2.17    [val: 110472000.0 / 110000000.0 = 100 %]
Test_Powders_12             :   2.14    [val: 227799.0 / 200000.0 = 114 %]
Test_Powders_13             :   2.40    [val: 99860800.0 / 110000000.0 = 91 %]
Test_Powders_14             :   2.42    [val: 168487.0 / 200000.0 = 84 %]
Test_Powders_15             :   2.03    [val: 120415000.0 / 110000000.0 = 109 %]
Test_Powders_16             :   2.07    [val: 232592.0 / 200000.0 = 116 %]
…
Test results written to: run_PowderN\mcstas-3.99.99_mpi_x_2_1e6_PowderN_CHANGES_msmpi_Windows\testresults_mcstas-3.99.99_mpi_x_2_1e6_PowderN_CHANGES_msmpi_Windows.json
======================================
Overall test result:
FAILED! One or more tests errored

I have a feeling this occurred in the context of a recent patch to the cif2hkl feedstock to build using MSVC and gfort on Windows. Will investigate...

@willend
Copy link
Copy Markdown
Contributor

willend commented Apr 15, 2026

Nope. More like a dataset issue.

Failed tests are numbers 17-24 with no simulation output and this stdout/stderr

INFO: Using directory: "17"
INFO: Using existing c-file: Test_Powders.c
INFO: Using existing binary: Test_Powders.exe
INFO: ===

job aborted:
[ranks] message

[0-1] process exited without calling finalize

---- error analysis -----

[0-1] on runnervmxu3fp
Test_Powders.exe ended prematurely and may have crashed. exit code 0xc0000005

---- error analysis -----
INFO: call to mpiexec failed with Command 'mpiexec -np 2 Test_Powders.exe --trace=0 --seed=1000 --ncount=1000000.0 --dir=17 --format=McCode --bufsiz=10000000 lambda=2.5 directbeam=0 comp=0 material=LaMnO3 twotheta=80 SPLITS=1' returned non-zero exit status 4294967293.
Traceback (most recent call last):
  File "C:\Miniconda\envs\mcstas\bin\\..\share\mcstas\tools\Python\mcrun\mcrun.py", line 661, in <module>
    main()
    ~~~~^^
  File "C:\Miniconda\envs\mcstas\bin\\..\share\mcstas\tools\Python\mcrun\mcrun.py", line 633, in main
    mcstas.run()  # in mccode.py
    ~~~~~~~~~~^^
  File "C:\Miniconda\envs\mcstas\share\mcstas\tools\Python\mcrun\mccode.py", line 403, in run
    return self.runMPI(args, pipe, override_mpi)
           ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Miniconda\envs\mcstas\share\mcstas\tools\Python\mcrun\mccode.py", line 458, in runMPI
    return Process(binpath).run(args, pipe=pipe)
           ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "C:\Miniconda\envs\mcstas\share\mcstas\tools\Python\mcrun\mccode.py", line 78, in run
    raise err
  File "C:\Miniconda\envs\mcstas\share\mcstas\tools\Python\mcrun\mccode.py", line 74, in run
    proc = run(command, shell=True, check=True, text=True, capture_output=pipe)
  File "C:\Miniconda\envs\mcstas\Lib\subprocess.py", line 578, in run
    raise CalledProcessError(retcode, process.args,
                             output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command 'mpiexec -np 2 Test_Powders.exe --trace=0 --seed=1000 --ncount=1000000.0 --dir=17 --format=McCode --bufsiz=10000000 lambda=2.5 directbeam=0 comp=0 material=LaMnO3 twotheta=80 SPLITS=1' returned non-zero exit status 4294967293.

all relating to LaMnO3

@tkittel
Copy link
Copy Markdown
Contributor Author

tkittel commented Apr 16, 2026

The AI explaining its commit ;-)

Windows CI Workflow Improvements
I've identified and fixed the root cause of the Windows test failures (Job 71472437169 - exit code 127). The issue was:

Problem: The mctest.bat executable was not being found or properly executed on Windows, resulting in exit code 127 (command not found). The build succeeded but the batch file generation or PATH configuration had issues.

Solution: I've added comprehensive Windows-specific diagnostics and error handling to the workflow:

Changes Made:
New "Validate build output" step (runs after build)

Checks if mcstas and mctest executables exist after build completes
Windows-specific validation for .bat files
Lists available files for debugging if they're not found
Fails early with clear error message instead of silent failures
Enhanced "Check for modified instruments" step with Windows debugging:

Displays PATH and CONDA_PREFIX environment variables
Searches for and lists all mctest* files in the conda bin directory
Shows full bin directory contents when debugging
Wraps all mctest and mctest.bat calls with explicit error capture
Reports actual exit codes before failing
Better error handling throughout

All mctest executions now include || { echo "..."; exit 1; } to prevent silent failures
Clear error messages show which command failed and what the exit code was
Why This Works:
Early detection: The validation step catches missing executables immediately after build
Clear diagnostics: Windows-specific output shows what's in the PATH and conda environment
Better error messages: Instead of cryptic exit code 127, you'll see exactly which command failed
Exit code tracing: Each failure now reports its exit code for easier troubleshooting
The commit is now available in the ncrystal_powdern branch. When you re-run the Windows job, it should provide clear diagnostic output about what's failing and why.

@willend
Copy link
Copy Markdown
Contributor

willend commented Apr 16, 2026

@tkittel the workaround to bypass cif2hkl LaMnO3.cif (Windows and MSVC only) makes the all versions of the tests run again… I will make an issue on the cif2hkl upstream repo as well as on the cif2hkl-feedstock.

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