Test NCrystal use with McStas

[willend]

• See if anything can be improved wrt. interoperability from a user perspective
• consider if our releases should include "latest NCrystal"

[tkittel]

This would obviously be great! I would like to discuss with you (perhaps in person tomorrow) how we can simultaneously get NCrystal into McStas, and still maintain the current (hackish?) manner of being able to use a fresh release of github with an older version of McStas. If nothing else, that would be useful for validation and development.

[willend]

I've added both mcpl and ncrystal as git submodules under /3rdparty in the mcstas repo. (Which should mean that future mcstas releases will "embed" the latest release of both of these codes... )

I will likely do as for mcpl, i.e. only embed the bare minimum and figure out how to package into our own infrastructure - but looks as if your CMakeLists is a little more complex than mine... So a little work will be needed there... ;-)

[tkittel]

Great, let me know if I need to do something in our CMakeLists to make it easier to work with :-)

For NCrystal, you for sure also need to package the data files (and avoid using the NCRYSTAL_DATADIR env var to point to them, but instead rely on the compile time flag - otherwise it will be difficult for users wanting to use a custom ncrystal installation on top of the one you ship). If possible I also highly recommend that you package the ncrystal_inspectfile command, since that is the main tool for users to be able to "visualise" their material configurations before actually starting simulations.

If it gets too much weird to understand, we can of course meet up and have a look in person.

[tkittel]

I assume ncrystal_inspectfile in mcstas will need ncrystal to support python3. I have opened mctools/ncrystal#6, it should hopefully be fairly quick to implement.

[tkittel]

I am releasing ncrystal 0.9.5 with python3 support shortly, so that should hopefully be one less hurdle in the path of ncrystal in mcstas :-)

[willend]

@tkittel I believe we now have a semi-automated approach for building/packaging NCrystal. - You are welcome to have a look at the CMakeFile

https://github.com/McStasMcXtrace/McCode/blob/master/mcstas-comps/libs/ncrystal/CMakeLists.txt

(Getting the "latest and greatest" is then achieved by running

git submodule init
git submodule update --recursive --remote

in a McCode tree -> gives local copies of the "full" distributions within the 3rdparty folder

[willend]

(Will actually take the liberty of closing this one, feel free to reopen at a later stage if bugs or inconsistencies are found...)

[tkittel]

Hi @willend ,

Sounds great! A few follow up questions for my education (hope you don't mind them here even though the issue was closed).

The cmake file you linked seemed familiar, did you do any particular changes? (perhaps setting examples+g4 to OFF at the top?).

It is hard for me to see, so a few perhaps stupid questions: Does everything work at runtime then after installing mcstas? I.e. is the ncrystal_inspectfile command available and working (i.e. ncrystal_inspectfile --test works fine and ncrystal_inspectfile Al_sg225.ncmat gives the expected gui popup?) and can one use the component in instrument files (without using ncrystal_preparemcstasdir first) ?

Cheers, Thomas

[willend]

That is exactly what I did:

diff 3rdparty/ncrystal/CMakeLists.txt mcstas-comps/libs/ncrystal/CMakeLists.txt 
1c1
< ##################################################################################
---
> #################################################################################
24,25c24,25
< set(BUILD_EXAMPLES ON CACHE STRING "Whether to build examples.")
< set(BUILD_G4HOOKS  ON CACHE STRING "Whether to build the G4 hooks if Geant4 is available.")
---
> set(BUILD_EXAMPLES OFF CACHE STRING "Whether to build examples.")
> set(BUILD_G4HOOKS  OFF CACHE STRING "Whether to build the G4 hooks if Geant4 is available.")

You are right, I should probably do a more rigorous test, but at least a locally built ncrystal_inspectfile Al_sg225.ncmat looked ok.

Comments here are of course OK

[tkittel]

Ok, thanks for the feedback. If you prefer to not editing the cmake file (to make future updates more transparent), you can of course invoke cmake with -DBUILD_EXAMPLES=OFF -DBUILD_G4HOOKS=OFF instead of editing the file (assuming you are invoking cmake directly?).

If ncrystal_inspectfile Al_sg225.ncmat seems to work, then I guess you are fine regarding the ncrystal library and C and python layers, as well as data file paths.

The final worry then would be if you edited the DEPENDENCY statement in the ncrystal sample component, to use the ncrystal installated with mcstas, and to not require the user to invoke ncrystal_preparemcstasdir.

[tkittel]

Btw., if you have some post-build tests you run to verify things, you could add ncrystal_inspectfile --test into the mix. It should test the ncrystal installation (not including the mcstas component).

[willend]

On 24 Apr 2018, at 13:45 , Thomas Kittelmann notifications@github.com<mailto:notifications@github.com> wrote:

Ok, thanks for the feedback. If you prefer to not editing the cmake file (to make future updates more transparent), you can of course invoke cmake with -DBUILD_EXAMPLES=OFF -DBUILD_G4HOOKS=OFF instead of editing the file (assuming you are invoking cmake directly?).

The thing is that we are indeed not invoking cmake directly. :-) (Moreover it may come in handy later to have a place for “further hacks”)

If ncrystal_inspectfile Al_sg225.ncmat seems to work, then I guess you are fine regarding the ncrystal library and C and python layers, as well as data file paths.

Yup.

The final worry then would be if you edited the DEPENDENCY statement in the ncrystal sample component, to use the ncrystal installated with mcstas, and to not require the user to invoke ncrystal_preparemcstasdir.

You are right, that ought to be fixed also. Likely via a configure_file in the above CMakeLists.txt

[willend]

Reopening as a little more work is needed for seamless integration

[tkittel]

Hi @willend

I was wondering what the current status is on this? I have to present it next week, so want to make sure I give the right impression (give me a call if you prefer a quick chat in person).

Cheers, Thomas

[willend]

Hi @tkittel

I am still missing the final bit about CMake-tuning my McStas-shipped sample component to use the McStas-shipped NCrystal component. It will be there for McStas 2.5 which is coming "soon"...

Sorry that I can't be more specific about a release date. :-)

Cheers, Peter

[willend]

@ebknudsen we now package NCrystal with McStas successfully at least on Debian. To try it out, please:

mcstas@mcstas-virtual-machine:~/NCryst$ mcstas-2.5dec121344-environment 

The new shell started here is now set up for running this version of mcstas:

McStas version 2.5dec121344 (Dec. 12, 2018)
Copyright (C) DTU Physics and Risoe National Laboratory, 1997-2018
Additions (C) Institut Laue Langevin, 2003-2018
All rights reserved

To end using this version of mcstas, exit this shell.

mcstas@mcstas-virtual-machine:~/NCryst$ source $MCSTAS/ncrystal/setup.sh
mcstas@mcstas-virtual-machine:~/NCryst$ ncrystal_preparemcstasdir 
Succesfully linked NCrystal_sample.comp to current directory and added NCrystalLink which is needed for instrument build.
mcstas@mcstas-virtual-machine:~/NCryst$ cp $MCSTAS/examples/NCrystal_example_mcstas.instr .
mcstas@mcstas-virtual-machine:~/NCryst$ mcrun -c NCrystal_example_mcstas.instr 

[climbcat]

This seems to work fine on a clean system. Output is:

mcstas@ubuntu:~$ mcrun -c NCrystal_example_mcstas.instr 
INFO: No output directory specified (--dir)
INFO: Using directory: "NCrystal_example_mcstas_20181212_051124"
INFO: Regenerating c-file: NCrystal_example_mcstas.c
CFLAGS= -Wl,-rpath,NCrystalLink/lib -LNCrystalLink/lib -lNCrystal -INCrystalLink/include
      INFO: Recompiling: ./NCrystal_example_mcstas.out
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp: In function ‘off_getBlocksIndex’:
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:3954:22: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp: In function ‘off_init’:
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4258:23: warning: format ‘%i’ expects argument of type ‘int’, but argument 5 has type ‘long int’ [-Wformat=]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4345:23: warning: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘long int’ [-Wformat=]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp: In function ‘off_display’:
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4556:24: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 3 has type ‘long int’ [-Wformat=]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4556:24: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 4 has type ‘long int’ [-Wformat=]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4556:24: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 5 has type ‘long int’ [-Wformat=]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp: In function ‘off_init’:
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4254:7: warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result [-Wunused-result]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4341:7: warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result [-Wunused-result]
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:4357:7: warning: ignoring return value of ‘fscanf’, declared with attribute warn_unused_result [-Wunused-result]
mccode-r.c: In function ‘mcdetector_import’:
mccode-r.c:767:51: warning: ‘ ’ directive output may be truncated writing 1 byte into a region of size between 0 and 1023 [-Wformat-truncation=]
In file included from /usr/include/stdio.h:862:0,
                 from mccode-r.h:42:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note: ‘__builtin___snprintf_chk’ output 9 or more bytes (assuming 1032) into a destination of size 1024
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp: In function ‘Monitor_nD_Init’:
/usr/share/mcstas/00test0019/tools/Python/mcrun/../mccodelib/../../../monitors/Monitor_nD.comp:636:29: warning: ‘%li’ directive writing between 1 and 20 bytes into a region of size between 0 and 127 [-Wformat-overflow=]
In file included from /usr/include/stdio.h:862:0,
                 from mccode-r.h:42:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:33:10: note: ‘__builtin___sprintf_chk’ output between 3 and 149 bytes into a destination of size 128
   return __builtin___sprintf_chk (__s, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       __bos (__s), __fmt, __va_arg_pack ());
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
INFO: ===
Instrument parameters for NCrystal_example (NCrystal_example_mcstas.instr)
[NCrystal_example] Initialize

Save [NCrystal_example]
Detector: powder_pattern_detc_I=6.46338e-11 powder_pattern_detc_ERR=1.9361e-12 powder_pattern_detc_N=1115 "powder_pattern_detc_1544620291.A"

Finally [NCrystal_example: NCrystal_example_mcstas_20181212_051124]. Time: 1 [s] 
INFO: Placing instr file copy NCrystal_example_mcstas.instr in dataset NCrystal_example_mcstas_20181212_051124

[tkittel]

Great! However, is the ncrystal_preparemcstasdir really still needed? I thought it was a hack introduced by us since we were not included properly in the release itself.

But probably good enough for 2.5, and actually I can see how it also postpones the problem of having the ability to use a future github release of NCrystal with an older McStas release, which could take a bit of debugging to get right otherwise (to prevent interference with the NCrystal version included with McStas).

After you source the NCrystal setup.sh, are you able to use ncrystal_inspectfile? E.g.

ncrystal_inspectfile --test
ncrystal_inspectfile Al_sg225.ncmat

[willend]

I agree that using ncrystal_preparemcstasdir is a bit of a hack, but I simply couldn't get around building / packaging the stuff properly otherwise... (And I think it is also a bonus to be as close as possible to how one would do if building from scratch...)

Wrt,. ncrystal_inspectfile this indeed works as long as the user has matplotlib for the used python (didn't dare test this on the Mac though)

[willend]

Have NCrystal packaged for the Mac now also - but using a hack. For whatever reason the procedure we usually use to package gives rise to @rpath issues - so I am simply using cmake / make / make install directly... See https://github.com/McStasMcXtrace/McCode/commit/1d9ca0e0e8183f04998e6abc3ac8ab4db94f3cf1

[willend]

And I dare not think of how one would currently make this work on windows - but they can always use WSL instead... ;-) Will keep this ticket open but remove from the https://github.com/McStasMcXtrace/McCode/projects/1 release board as I think what we have is acceptable

[willend]

I think what we currently have is acceptable, but sticking this on the board for 2.5.1 just in case...

[willend]

I just retested for 2.6, the above solution still works OK. I will revisit the web-based crystal instructions to check that what is written there for use with McStas reflects this thread.

Closing